[Geany-devel] encoding combo boxes bug - utility functions

[Matthew Brush]

On 01/23/2012 08:30 AM, Nick Treleaven wrote:
> On 22/01/2012 22:00, Lex Trotman wrote:
>> When working with a common well known library like GTK it is better to
>> use the well known interface directly rather than creating private
>> partial wrappers.
>>
>> Contributors who know GTK don't have to learn the private interface
>> (or complain about what is missing, or just use GTK directly anyway
>> since they don't know about the private interface) and contributors
>> who don't know GTK can learn an interface that is useful to them
>> elsewhere, rather than one that just works in Geany.
>
> You make a valid point, but most contributions are from the core team
> that know our utility functions. In this case we're discussing a fairly
> trivial function, but if it gets used 10 or 50 times in the code base

I probably don't know 40%+ of Geany's code after casually hacking on it 
for well over a year. When reading the code, I have to refer to the 
source file for each function called to see what it does, with GTK+ I've 
already done this for many cases, and know what it does. When writing 
the code, I have to first write it in normal GTK+ and then go through 
and make sure I haven't used any functions that are wrapped in the Geany 
API/headers and even other static functions in the same file. It sounds 
trivial if you are intimate with the source code, but if you aren't it 
can make understanding the code you need to understand in order to fix a 
bug or add a feature that much harder to follow.

> then that's a significant benefit in avoiding temporary variables or
> nested expressions, which are harder to read. As I said, if the function
> is obvious, there's no harm.
>

You don't really avoid temp vars, you just put them in another file. And 
if an expression is only nested one or two levels deep, it's easier (at 
least for me) in many cases to read if the code is inline.

For a (fictional) example:

     void some_func(void)
     {
       GError *err=NULL;

       if (!g_some_func(..., &err))
       {
         printf ("error: %s\n", err->message);
         g_error_free (err->message);
       }
       ...
     }

is easier for me to read than:

     /* misc.h */
     #define EZG(...) { ... actual code from above ... }

---- separate files ----

     /* some.c */
     void some_func(void)
     {
       EZG(g_some_func, ...);
       ...
     }

Even if it saves you repeating that same 5-6 line idiom a thousand times 
throughout the source, unless you wrote both pieces of code, or unless 
EZG() is in a well know API like GTK+, then it makes the code harder to 
read, IMO, which many more people do many more times than writing it.

> In other cases we have functions that save 10 lines of code per call.
> This is a massive help that outweighs having to work out what the
> function does.
>

+1.

> Another point is that exposing Matt's ui_get_builder before we actually
> have code that needs it seems premature. We already know we need to
> lookup objects though, and that a short syntax is needed.
>

It's the same thing, you still expose one function to use, but with 
ui_get_builder() you get the entirety of the GtkBuilder API for free and 
never have to add another wrapper function for it. If you have 
ui_builder_get_object(), if you need another function from the 
GtkBuilder API, then you need to add another function, like 
ui_builder_add_object(). Now you have two specialty functions functions, 
both wrapping ones that are already included with gtk.h.

But anyway, the current function is so trivial, and I know everyone has 
a different preference about these things, it's just my two cents...

Cheers,
Matthew Brush