On 05/09/2013 09:41, Matthew Brush wrote:
On 13-09-04 09:20 AM, Nick Treleaven wrote:
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.
Most files already include document.h.
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.
We don't support accessing multiple windows IIUC.
BTW I was just providing code for that specific case, not necessarily
for all iteration.
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
}
Yes, that is much better if we have C99/C++. That also solves most of
the points above.
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.
Yes. Although sometimes bugs do slip through, even in incrementing for
loops. Suppose someone used <= instead of <, or forgot to initialize i.
10. The macro should be named `foreach_document_index()`.
OK.
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.
It would be useful, but also a distraction for the core project.
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
