Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260) Bug#58918

[Øystein Grøvlen]

On 12/15/10 01:38 PM, Martin Hansson wrote:
> Tor Didriksen skrev 2010-12-15 12.42:
>> On Tue, Dec 14, 2010 at 1:50 PM, Martin Hansson
>> <martin.hansson@stripped>wrote:
>>
>>> #At file:///data0/martin/bzrroot/refactoring-Field_clone/n-mr-o-t/
>>> based on
>>> revid:tor.didriksen@stripped
>>>
>>> 3260 Martin Hansson 2010-12-14
>>> Bug#58918: Add a clone member function to the Field class for better
>>> duplication
>>>
>>> The patch removes all copying of Field objects using memcpy() and
>>> replaces it
>>> with virtual clone() and clone(MEM_ROOT*) member functions that rely
>>> on the
>>> copy constructor. An operator new (MEM_ROOT*) is also implemented. The
>>> sole
>>> purpose of the Field::size_of() member function was to support the
>>> memcpy and
>>> it is now removed.
>>> @ sql/field.cc
>>> Removal of memcpy() based duplication.
>>> @ sql/field.h
>>> New implementation of clone() member functions.
>>> Removal of size_of() member functions
>>> @ sql/item.cc
>>> Removal of memcpy() based duplication.
>>> @@ -84,8 +79,12 @@ class Field
>>> Field(const Item&); /* Prevent use of these */
>>> void operator=(Field&);
>>> public:
>>> + /* To do: inherit Sql_alloc and get these for free */
>>>
>>>
>> Why not go all the way :-)
>> This works like a charm:
>>
>> class Field : public Sql_alloc
>> {
>> Field(const Item&); /* Prevent use of these */
>> void operator=(Field&);
>> public:
>> uchar *ptr; // Position to field in record
>> ...
>> ...
> I am planning to. In case you didn't notice there's a comment about it.
> The reason I don't want to do it is because it would work by
> coincidence, not by design. The class Sql_alloc is -very wrongfully -
> defined in sql_list.h. We slip that include file in sideways by
> including mysqld.h which in turn includes sql_list.h. I plan as my next
> refactoring to create a file sql_alloc.h and then make every class that
> implements an identical 'new' operator (quite a few of those actually)
> inherit Sql_alloc instead. Only then will I have gone all the way :-)
>
> If you insist, of course I can do it!
>

Public inheritance?  I think it is questionable whether this is an is-a 
relationship.  (Ref. "the single most important rule in object-oriented 
programming with C++" according to Scott Meyers)

-- 
Øystein