> - the UCNV prefix is not necessary, we are in the UConverter class
> already, I would simply use LATIN_1, or ILLEGAL for exampe.
> - I would add ERROR_ to the error related constants (UNASSIGNED & co), or?
>
For the UConverterType enum I could maybe get behind that.  For
UConverterCallbackReason I think we need something, but ERROR_ isn't
universally applicable since three are just status incidents
(UCNV_RESET, UCNV_CLOSE, UCNV_CLONE).  Perhaps: REASON_* for these?
In that vein, I'd be tempted to make the UConverterType values be
TYPE_*

> - It is OO, we should or could use exception. Warning are not nice to
> deal with and errors are quite common during conversion
>
We are.  UConverterException is thrown everywhere but one place.  That
one exception is the static method I added to give a non-oop-like API
for the most core functionality.  Given its non-oopness, I left it
non-exceptiony.  See Gustvo's notes about error handling below.

> Some questions, is 'latin1' the same than const UCNV_LATIN_1? or is
> there any relation between the two?
>
The UCNV_LATIN1 constant is a numeric value, so they're not the same,
no.  The relationship is that when you instantiate a UConverter with
'latin1' (or 'iso-8859-1', or any of its aliases),
$conv->getSourceType() (or getDestinationType() as appropriate) will
return UConverter::UCNV_LATIN_1.

Each type in the UConverterType set uses a different algorithm
approach to do the character conversion.  Sets like latin1, utf*,
ascii, etc... can be done faster than sets like 'koi8-r' because the
conversions are simple bitshifts (very simple in the case of
ascii/latin1).  This just surfaces some info on what the converter is
doing internally and isn't much use beyond a bit of logging, in
practice.

> btw, can you add the changes to config.w32 too pls?
>
Ah, right sorry, had meant to do that at the last minute.  Will add
that in a bit,

> Error handling is done in a different way from the rest of the extension.
> Unusual as this may be, it would be a bad idea to introduce inconsistency
> here. See the implementations in the other modules.
>
I do agree, I was hoping I could slip that one by. :p  I'll take
another look at the rest of ext/intl and see about adapting my error
handling.

> The tests have poor coverage.
>
The important bits are hit, but yeah, it could stand to have better
testing.  I'll add some.

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to