[webkit-efl] FW: Use of pimpl idiom for Ewk classes

[Kangil Han]

Oh.. I was confused. Maybe I so much focused on OOP. ;)
Yes, we shouldn't make another layer that burdens on client side.
 
As you said, Ewk classes should be C code compatible.
For this, direct access will be needed. Agreed.
 
Please don't get me wrong.
I am a passionate OOP advocate. lol
I just wanted to check out all aspects from this change since once landed
then other many developers will spend time to take a look at pimpl idioms
structure and information for debug as I've been doing.
 
Anyway, now LGTM on my side.
My last comment is please document in detail to help out EFL developers
being get used at this. :)
 
Many thanks,
Kangil
 
From: Dumez, Christophe [mailto:christophe.dumez at intel.com] 
Sent: Friday, October 19, 2012 8:28 PM
To: kangil.han at samsung.com
Cc: Thiago Marcos P. Santos; webkit-efl at lists.webkit.org; Kenneth R
Christiansen; cpgs at samsung.com
Subject: Re: FW: [webkit-efl] Use of pimpl idiom for Ewk classes
 
Hi,

I forgot to mention that "String vs const char*" is just one example and may
not be the best one because they are strongly related. In your example, you
could use eina_stringshare_add() or strdup() to avoid the crash and convert.
The difference is on client (browser) side because they now need to free()
or eina_stringshare_del() explicitly.

If you start thinking about "Vector vs Eina_List*" then it becomes
impossible to efficiently convert between the 2 and you will need 2 C++
methods to return both types.

Kr,
On Fri, Oct 19, 2012 at 2:21 PM, Dumez, Christophe
<christophe.dumez at intel.com> wrote:
> You can already see it in my patch for the destination() getter. It has to
return a "const char*" instead of a String because it is used by the C API.
IMHO, external object(s) should treat Ewk classes as C++ as much as possible
in this case.
Therefore, we don't have to make functions that return char*.
Rather, caller can use utilization function to change String class to char*.
 
--- Ewk class
String destination() { . }
--- Utilization function
char* StoCP(String s)
{
    char* cp = s.utf8().data();
    return cp;
}
--- C layer
void i_am_c_public_api()
{
    char* cp = StoCP(destination);
}
 
Eh what? This is wrong and will crash.

char* StoCP(String s) {
    char* cp = s.utf8().data(); // s.utf8() returns a temporary CString
object and you store a pointer to the internal representation of that
temporary CString object
    return cp; // You return a pointer to an area in memory that is no
longer valid because it was temporary.
}

Secondly, even if that worked, we loose all benefit of Eina Stringsharing
since we would return raw char* to the client.

Kr,
-- 
Christophe Dumez
Linux Software Engineer, PhD
Intel Finland Oy - Open Source Technology Center



-- 
Christophe Dumez
Linux Software Engineer, PhD
Intel Finland Oy - Open Source Technology Center
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-efl/attachments/20121019/dc87cffc/attachment-0001.html>