jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
Subject Re: Converter#getCalendar(Object, Calendar)
Date Wed, 31 Dec 2014 14:49:44 GMT
Am 28.12.2014 um 23:53 schrieb sebb:
> On 28 December 2014 at 20:34, Philippe Mouawad
> <philippe.mouawad@gmail.com> wrote:
>> On Sun, Dec 28, 2014 at 4:33 PM, Felix Schumacher <
>> felix.schumacher@internetallee.de> wrote:
>>
>>> Hi all,
>>>
>>> I have two questions about getCalendar(Object, Calendar) in Converter.
>>>
>>> 1)
>>>   Documentation says the default value would be the current date, while the
>>> code shows it will be defaultValue (second parameter). I will correct the
>>> documentation. (Well not really a question then :)
>>>
>> OK
> +1
Done. Together with the other documentation commits we are now down to 
only 460 errors due to missing javadoc param, return or throws attributes :)
>
>>> 2)
>>>   The code is quite deep because of the exception handling. I think it
>>> would be nicer to use a for loop to iterate the possible formats. Should I
>>> do so? (Now that is a question)
>>>
>> Yes, but as you say in 3) code seems a bit weird
> -1
>
> I would leave well alone.
>
>>> 3)
>>>   While the first (really the second) conversion uses date.toString() to
>>> convert date to a String, the following conversions assume that date is a
>>> String (using (String) date casts). That would be handled by a correct
>>> implementation for 2), or could be handled separately. Should I do so? (If
>>> so, by using for loop for 2, or just a simple toString() call?)
>>>
>> Code seems buggy or at least not clear for me, looking at callers it seems
>> parameter can be date or string. So if it's not date, it is string and I
>> don't understand in this case the toString
> -1
>
> I don't think it's a good idea to tamper with this method.
> There are no unit tests for it, and it is used for preparing the TestBeans.
>
> If any subtle changes are introduced it may prove very difficult to debug.
Very well, the unit tests are there already :) But as you are against 
it, I will not submit the changes. However if anyone likes to look at 
them, I have attached them.

Regards
  Felix
>
>>> Regards
>>>   Felix
>>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.


Mime
View raw message