Re: [PEAR-DEV] [PEPr] +1 for Date and Time::Date_HumanDiff

On 2012-06-04 11:58, Christian Weiske wrote:
> Hello Michael,
>
>
>> Michael Gauthier (http://pear.php.net/user/gauthierm) has voted +1 on
>> the proposal for Date and Time::Date_HumanDiff.
>> Vote information:
>> http://pear.php.net/pepr/pepr-vote-show.php?id=678&handle=gauthierm
>
> Thanks for the extensive review.
>
You're welcome. :)

You've addressed all my concerns and my vote is no longer conditional.

Cheers,
Mike

>
>> 1.) Method name setLocale() instead of setLanguage() makes more sense
>> to me as you are passing in locales. I'd rename Lang classes as
>> Locale at the same time.
> Done.
>
>> 2.) Does the sort method need to be static? I'd make it protected and
>> dynamic so it can be overridden in subclasses.
> Done.
>
>> 3.) In makeTimestamp you use if ($foo) { return 'a'; } else { return
>> 'b'; }. The else is redundant. Two separate if statements might be
>> clearer.
> Done.
>
>> 4.) isIncludable() has error suppression operator which is not
>> allowed in PEAR. Can you use file_exists + is_readable instead?
> No, sorry. I need to see if the file exists in the include path, and
> using fopen + its third parameter is the pure php implementation of
> that functionality.
>
>> 5.) Class docs for the Lang classes would be nice.
> Done.
>
>> 6.) What about languages with multiple plural forms? Will this work
>> with Slocak or Arabic? Should this library even be responsible for
>> translating strings? Another library like gettext or Zend_Translate
>> seems like a better place to handle that.
> Good catch. I'm passing the number now to the locale classes, which
> means that the implementation is completely free. Gettext and
> Zend_Translate backends are possible now.
>
>
>