replace iconv with mb_convert_encoding

[connorhu]

The require-dev does not include the ext-iconv requirement, but tests use it. Since there is only one place with iconv call, I replaced it with mb_convert_encoding instead. mb-string require is there anyway.

[thirsch]

There are two more occurrences of iconv:

• https://github.com/FriendsOfSymfony1/doctrine1/blob/master/lib/Doctrine/Parser/Xml.php#L87
• https://github.com/FriendsOfSymfony1/doctrine1/blob/master/lib/Doctrine/Search/Analyzer/Utf8.php#L46

At least one argument is a variable, there it might be a BC as seen in here (ISO8859-15 vs. ISO-8859-15)?

[connorhu]

There are two more occurrences of iconv:

* https://github.com/FriendsOfSymfony1/doctrine1/blob/master/lib/Doctrine/Parser/Xml.php#L87

* https://github.com/FriendsOfSymfony1/doctrine1/blob/master/lib/Doctrine/Search/Analyzer/Utf8.php#L46

At least one argument is a variable, there it might be a BC as seen in here (ISO8859-15 vs. ISO-8859-15)?

I did not even check the source. However, we should also include ext-iconv.

[connorhu]

I was thinking. If the encoding is in a simple format then we can still do the ISO8859-15 => ISO-8859-15 conversion, but with iconv we can specify quite messed up variants (ascii//translit and others).

[thirsch]

I was thinking. If the encoding is in a simple format then we can still do the ISO8859-15 => ISO-8859-15 conversion, but with iconv we can specify quite messed up variants (ascii//translit and others).

My initial thoughts: Maybe it should just be added to the composer.json as it currently is a requirement and the code would crash if iconv is not installed. I'm not sure if it's worth the effort to change it.

[connorhu]

I don't see why it failed, but the PR seems good.

[thirsch]

Looks like just bad timing...

The model under test uses the timestampable template and renames the created-field to event_date. Before time() is called later in the test, the model is created and saved. If the creation happens in the second before getting the current timestamp, the test condition will fail. It would be safer to test ($time => $now - 1).

Here is the test:

$elem = new Ticket_1325_TableName_NoAlias();
$elem->id = 1;
$elem->save();  // <-- here the current timestamp will be used (e. g. 1706130426)

$res = Doctrine_Query::create()
    ->from('Ticket_1325_TableName_NoAlias')
    ->fetchOne(array(), Doctrine_Core::HYDRATE_ARRAY);

$now = time();  // <-- the reference timestamp will be set here (e. g. 1706130427)
$time = strtotime($res['event_date']);
$this->assertTrue(($now + 5 >= $time) && ($time >= $now));

[thirsch]

[...] It would be safer to test ($time => $now - 1).

Or even better to move $now = time() up to determine the current timestamp before save() is called. :)