List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:December 20 2010 9:46am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)
Bug#58918
View as plain text  
On 12/20/10 09:51 AM, Martin Hansson wrote:
> Øystein Grøvlen skrev 2010-12-17 13.28:
>> Hi Martin,
>>
>> Thanks for a good initiative. I have bitten by this memcpy stuff
>> myself. I wonder whether I should require or just suggest that you try
>> to add a unit test for your clone methods. Unit tests for basic
>> classes like Field should be possible without too much work. (Tor told
>> me it was pretty straight-forward for Item_int).
> It could of course be interesting to get into the unit testing
> framework, I haven't had time for that in the past. But I am not sure
> what I should be testing. Any suggestions? The only thing that needs to
> be tested IMHO is that every clone method returns an object of precisely
> the class for which is it's called and not a descendant class. But to
> test that requires RTTI - which we don't have - doesn't it?
>
> Should I just instantiate every Field subclass and call clone on it?

Yes.  In addition, you could call eq() and eq_def() to check that they 
are actually equal. (Since clone does a shallow copy, I assume eq() and 
eq_def() should return true.)

>>
>> I also think that we should drop the existing clone method:
>>
>> > Field *clone(MEM_ROOT *mem_root, TABLE *new_table);
>>
>> This is used in only one place, and its implementation uses only
>> public Field methods, so I think the entire code should be moved to
>> the caller.
>> (The Field interface is unecessarily large. No need to provide
>> unecessary helper functions.)
> Sure, I'll do that. I didn't look into this at all. Attached please find
> a patch for only this. Please scrutinize it.

Looks very good.

Thanks,

--
Øystein

>
> Best Regards
>
> Martin
Thread
bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260) Bug#58918Martin Hansson14 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen15 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson15 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen16 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen16 Dec
      • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson16 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen16 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson16 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen17 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson20 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen20 Dec