On 05/07/2008 01:20 PM Derick Rethans wrote: > On Wed, 7 May 2008, Tobias Schlitt wrote: >> On 05/07/2008 01:06 PM Derick Rethans wrote: >>> On Wed, 7 May 2008, Alexandru Stanoi wrote:
>>>> - ezcFeedTools::prepareDate() should not accept DateTime objects. If it >>>> receives them, this indicates bad code. >>>> + >>>> +# What if you assign a DateTime object to $feed->published for example? >>>> Should >>>> + the code throw an exception because the date was not a string or >>>> timestamp? >>> DateTime objects should be supported here. How is it bad code if it >>> receives them? >> If you already have a DateTime object, you should not call this method. >> Its not good to just call a function and don't actually know what type >> you currently have. Parsing whatever date value (string or timestamp) is >> ok, but if you already parsed the date and try to parse it again, that >> is not good. > I disagree here. This method is called to accept whatever form of input, > and make sure the returned data is a DateTime object. The method is > automatically called when you set $feed->published and it makes much > more sense to have this method handle *all* input instead of making > another exclusion before the function is called. THis also makes it > possible to support multiple types of DateTime objects if those would be > added later. Having such a behaviour inside __set() is good. However, providing an external method for it looks dirty to me. This could a) seduce people to start coding the "give me whatever you have, I'll return the correct value" way in other cases and b) lead to unecessary calls to this method in other places, just to ensure that a DateTime object is there, no matter if there already is one. The re-usable part of the method is the parsing behaviour, not the passthrough of DateTime objects. It also makes a more clean API, if only a scalar is accepted as input and a DateTime object is accepted as output. Therefore I'd recommend to move the exception for that a DateTime object is submitted to the actual __set() method and just let this method do what it indicates: Parsing another date format to a DateTime object. Regards, Toby -- Mit freundlichen Grüßen / Med vennlig hilsen / With kind regards Tobias Schlitt (GPG: 0xC462BC14) eZ Components Developer [EMAIL PROTECTED] | eZ Systems AS | ez.no -- Components mailing list [email protected] http://lists.ez.no/mailman/listinfo/components
