Re: [webkit-dev] free functions

2010-06-10 Thread Maciej Stachowiak

On Jun 3, 2010, at 1:36 AM, Chris Jerdonek wrote:

 On Tue, May 25, 2010 at 10:01 AM, Darin Adler da...@apple.com wrote:
 On May 25, 2010, at 7:54 AM, Chris Jerdonek wrote:
 
 I sometimes come across public member functions whose implementations do 
 not depend on private data.
 
 There is a school of thought that such functions are better non-member 
 because it reduces the number of functions coupled to private data. On the 
 other hand, I've heard the argument that making such functions free creates 
 naming issues -- it's not clear to the caller in which header file to find 
 the free function.
 
 My question for WebKit is whether naming considerations outweigh 
 encapsulation considerations.  And if so, is there a naming convention or 
 otherwise that we can use to make finding free functions easier?
 
 We do need our classes to be smaller so we can understand the structure of 
 the code. The encapsulation benefits of having a much smaller number of 
 members in a class are well worth some cost. But there are at least two 
 considerations that come into play when replacing a member function with a 
 free function:
 
1) Free functions still have to go in some header/source file. The usual 
 rule for finding a function is to look for a file named based on the class. 
 Without a class name we have to do something to make it practical to find 
 the functions in the source tree without a lot of searching.
 
2) Free functions need names that are clear and unambiguous with no 
 context other than the WebCore namespace. We try to keep member function 
 names short, and we can do so in part because they have a class name 
 context. The same function as a free function will almost certainly need a 
 longer name. Each time we create a free function we have to think about what 
 an appropriate name is; it’s a mistake to leave the same short name that was 
 previously used for a class member.
 
 Another possible way to get encapsulation benefits with fewer of the other 
 problems is to group functions into classes or namespaces that have no data 
 and nothing else private. This may be helpful if the class or namespace name 
 has a good name with a clear concept.
 
 
 Would the following simple convention be an acceptable option?  A free
 function in a header file could go in a nested namespace whose name is
 the name of the header file followed by Functions (as in free
 functions).  An example in Chrome.h might be--
 
 WebCore::ChromeFunctions::applyWindowFeatures(Chrome*, const WindowFeatures);
 
 Would adding such a non-member function be preferable to adding to the
 Chrome class a public member function that didn't depend on private
 Chrome data?  The encapsulation discussion above suggests it would.
 
 I'm just trying to think of a simple alternative so the default need
 not always be to add another member function.
 
 For comparison, we have essentially 8 files whose file name ends in 
 Functions:
 
 JavaScriptCore/API/JSCallbackObjectFunctions.h
 JavaScriptCore/runtime/JSGlobalObjectFunctions.*
 JavaScriptCore/wtf/HashFunctions.h
 JavaScriptCore/wtf/StringHashFunctions.h
 WebCore/bindings/js/JSPluginElementFunctions.*
 WebCore/dom/PositionCreationFunctions.h
 WebCore/xml/XPathFunctions.*
 WebKit/mac/Plugins/WebNetscapeDeprecatedFunctions.*

I just discussed this topic with Darin briefly in person. We both agreed that, 
in general, free functions do not need a special namespace, an overly specific 
name, or a separate header. Free functions that are closely related to a class 
can be thought of as part of the interface exposed by that class - it's just a 
part that's not necessarily core functionality, and that doesn't need access to 
class internals. Going back to your specific example,

I would just do:

namespace WebCore {
void applyWindowFeatures(Chrome*, const WindowFeatures);
}

Due to C++ overloading, it doesn't matter much if some other class has an 
applyWindowFeatures function. C++ will resolve the namespace collision. The 
main question to consider is whether it's still clear at the call site:

chrome-applyWindowFeatures(feature);

would change to:

applyWindowFeatures(chrome, feature);

That's likely to still be understandable. And in this particular case, the 
function name is unlikely to be ambiguous.

I would also suggest that in most cases, free functions closely related to a 
specific class should generally go in the same header. Exceptions would be:

(a) Sets of functions that are related to each other but aren't closely related 
to a single specific class (for example hash functions for a bunch of different 
types).
(b) Functions that comprise clearly separate subsystem, and are not truly about 
the main class they work with. For example, a set of functions to parse email 
addresses might operate mainly on strings, but they are not conceptually part 
of the interface of String, they just happen to use it.
(c) Functions that pull in a bunch of extra header dependencies, but are 

Re: [webkit-dev] free functions

2010-06-03 Thread Chris Jerdonek
On Tue, May 25, 2010 at 10:01 AM, Darin Adler da...@apple.com wrote:
 On May 25, 2010, at 7:54 AM, Chris Jerdonek wrote:

 I sometimes come across public member functions whose implementations do not 
 depend on private data.

 There is a school of thought that such functions are better non-member 
 because it reduces the number of functions coupled to private data. On the 
 other hand, I've heard the argument that making such functions free creates 
 naming issues -- it's not clear to the caller in which header file to find 
 the free function.

 My question for WebKit is whether naming considerations outweigh 
 encapsulation considerations.  And if so, is there a naming convention or 
 otherwise that we can use to make finding free functions easier?

 We do need our classes to be smaller so we can understand the structure of 
 the code. The encapsulation benefits of having a much smaller number of 
 members in a class are well worth some cost. But there are at least two 
 considerations that come into play when replacing a member function with a 
 free function:

    1) Free functions still have to go in some header/source file. The usual 
 rule for finding a function is to look for a file named based on the class. 
 Without a class name we have to do something to make it practical to find the 
 functions in the source tree without a lot of searching.

    2) Free functions need names that are clear and unambiguous with no 
 context other than the WebCore namespace. We try to keep member function 
 names short, and we can do so in part because they have a class name context. 
 The same function as a free function will almost certainly need a longer 
 name. Each time we create a free function we have to think about what an 
 appropriate name is; it’s a mistake to leave the same short name that was 
 previously used for a class member.

 Another possible way to get encapsulation benefits with fewer of the other 
 problems is to group functions into classes or namespaces that have no data 
 and nothing else private. This may be helpful if the class or namespace name 
 has a good name with a clear concept.


Would the following simple convention be an acceptable option?  A free
function in a header file could go in a nested namespace whose name is
the name of the header file followed by Functions (as in free
functions).  An example in Chrome.h might be--

WebCore::ChromeFunctions::applyWindowFeatures(Chrome*, const WindowFeatures);

Would adding such a non-member function be preferable to adding to the
Chrome class a public member function that didn't depend on private
Chrome data?  The encapsulation discussion above suggests it would.

I'm just trying to think of a simple alternative so the default need
not always be to add another member function.

For comparison, we have essentially 8 files whose file name ends in Functions:

JavaScriptCore/API/JSCallbackObjectFunctions.h
JavaScriptCore/runtime/JSGlobalObjectFunctions.*
JavaScriptCore/wtf/HashFunctions.h
JavaScriptCore/wtf/StringHashFunctions.h
WebCore/bindings/js/JSPluginElementFunctions.*
WebCore/dom/PositionCreationFunctions.h
WebCore/xml/XPathFunctions.*
WebKit/mac/Plugins/WebNetscapeDeprecatedFunctions.*

(For files like these, we would probably not want to use a convention
like the above.)

--Chris
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] free functions

2010-05-25 Thread Darin Adler
On May 25, 2010, at 7:54 AM, Chris Jerdonek wrote:

 I sometimes come across public member functions whose implementations do not 
 depend on private data.
 
 There is a school of thought that such functions are better non-member 
 because it reduces the number of functions coupled to private data. On the 
 other hand, I've heard the argument that making such functions free creates 
 naming issues -- it's not clear to the caller in which header file to find 
 the free function.
 
 My question for WebKit is whether naming considerations outweigh 
 encapsulation considerations.  And if so, is there a naming convention or 
 otherwise that we can use to make finding free functions easier?

We do need our classes to be smaller so we can understand the structure of the 
code. The encapsulation benefits of having a much smaller number of members in 
a class are well worth some cost. But there are at least two considerations 
that come into play when replacing a member function with a free function:

1) Free functions still have to go in some header/source file. The usual 
rule for finding a function is to look for a file named based on the class. 
Without a class name we have to do something to make it practical to find the 
functions in the source tree without a lot of searching.

2) Free functions need names that are clear and unambiguous with no context 
other than the WebCore namespace. We try to keep member function names short, 
and we can do so in part because they have a class name context. The same 
function as a free function will almost certainly need a longer name. Each time 
we create a free function we have to think about what an appropriate name is; 
it’s a mistake to leave the same short name that was previously used for a 
class member.

Another possible way to get encapsulation benefits with fewer of the other 
problems is to group functions into classes or namespaces that have no data and 
nothing else private. This may be helpful if the class or namespace name has a 
good name with a clear concept.

I also think that it’s fine for free functions to have longer names than member 
functions.

Functions like the ones in SuddenTermination.h, LinkHash.h, UUID.h, and even 
markup.h seem to be OK as free functions, so I think we can definitely deal 
with these issues and use them more. But it’s something I think we have to do 
carefully with sensitivity until we get a little more experience with it.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev