[Geany-Devel] Let's move to C++98 - Re: Lets move to C99

[Matthew Brush]

On 13-09-04 09:20 AM, Nick Treleaven wrote:
> On 01/09/2013 02:36, Matthew Brush wrote:
>>>> I would even go so far as to say it's silly to not use C++11 since it's
>>>> [...]
>>>
>>
>> Just for fun I wrote some theoretical code that could be used in a
>> program like Geany to compare styles between various C's and C++'s
>> (note: none of it tested).
>>
>> http://pastebin.geany.org/gYFMO/
>
> My C89 & C++ version:
>
> guint i;
> foreach_document(i) {
>      GeanyDocument *doc = documents[i];
>      // do stuff with every valid doc
> }
>

While this code is short, it's actually sort of nuts too (and also not 
very C++-ish).

1. You should include the definition of the custom macro used and the 
definitions of the two other non-obvious macros used inside the first 
macro for a fair comparison, since none of them are standard or even 
well-known like the pasted examples.

2. The code does not loop over the windows like the pasted code, so 
you'd need another outer loop which would likely have a 2nd style of 
iteration wrapped around the existing loop.

3. It's somewhat unclear what type should be passed in looking at the 
macro without the doc comments (I always forget and have to RTFM each 
time I see it since we often use gint where we mean guint).

4. The argument is not only assigned to but also evaluated multiple 
times (what if you pass `--i` or something weird as argument). I guess 
assigning might be good here since the compiler would catch it rather 
than flying off the end of the array when it hits `(guint)-1` (UINT_MAX) 
on underflow.

5. It's fairly tricky to debug misuses of the macro (better with Clang).

6. The implicit check in the loop (failure first) indexes into a macro 
wrapper around a whole other type using a hard cast from the result of 
another function-like macro cast thing off in another file, and is 
dereferenced without any NULL check (although probably safe in this 
case, unless a plugin or something re-assigns the global variable or 
redefines any macro aliases of it).

7. It forces you to define an indexing variable outside of the loop into 
the wrong scope (C89-style, even if used in > C99 plugin code).

8. Even though slightly dirtier, it would be more useful to iterate over 
document pointers than document indexes (what are those? tab pages? 
order of opening? arbitrary array indices?), for example (untested, C99 
or GNU89 probably):

     #define foreach_doc(doc) \
         for (unsigned z_=0; z_ < documents_array->len; z_++) \
            if ( !((doc) = documents_array->pdata[z_]) || \
                 !(doc)->is_valid ) { continue; } else
     ...
     GeanyDocument *doc;
     foreach_doc(doc) {
       // every valid doc
     }

9. I personally find it annoying to use API's that have all their own 
versions of things that are really common and not that hard anyway; it's 
difficult to learn them all, and also all the stuff mentioned above when 
they are macros. Consider this slightly longer example that uses no 
macro madness and common C looping idiom:

     guint i;
     for (i = 0; i < documents_array->len; i++) {
       GeanyDocument *doc = documents_array->pdata[i];
       // do stuff with every *should be* (but isn't) valid doc
     }

Assuming the API didn't have the weirdness about documents being invalid 
but still sticking around for whatever reason, it's the same number of 
lines as the macro version and no magic.

10. The macro should be named `foreach_document_index()`.

</macro-hate>

> I don't think we should rewrite Geany's API in C++, or maintain a C++
> wrapper for the C one, except for any cases which are particularly
> bug-prone.
>

An idiomatic C++ binding would be quite useful for a plugin written in 
normal C++. In my current C++ plugin I'm working on, I'm actually 
wrapping up a few parts of Geany's API it uses to avoid scattering the 
weird C code containing lots of raw pointers, miscellaneous allocators, 
different string/collection types, and so on, throughout the normal C++ 
code.

Cheers,
Matthew Brush