On 10/04/2011 11:12, Artyom wrote:
line 150: I think sresult.append(out_start, out_ptr); would be slightly clearer.
I should look on it carefully whether I can change this.
iconv API is very gentle in how it handles its pointers and states so I need to do everything carefully.
Also I assume that append(Begin,End) may be less efficient then append(pointer,size) as it may be naively implemented as
while(begin!=end) s += *begin++;
Why make such assumptions that could very well be unfounded when it only makes your code harder to understand?
iconv_between: Inheritance from iconv_from_utf is a little odd.
But it makes a code much simpler.
Inheritance should not be used for code sharing unless used with the CRTP idiom.
iconv_to_utf: The comments from iconv_from_utf apply. Also, the implementation is almost the same as iconv_from_utf. I think you could define a single function template to implement both.
Not exactly. I had found that it is much safer and readable to have two implementations then to create quite complex meta programming to make sure it works.
It is much easier to assume that at least one side is "char"
Code duplication is one of the problems with your library in my opinion. More on that in my upcoming review. So it is intentional that you duplicate code because you find writing it once generically makes it hard to validate?