On 22-1-2014 21:25, mathog wrote:

Sometimes it is hard to shake off decades of C programming, so please bear with.

In src/display/drawing-context.h near the top one finds:

class DrawingContext : boost::noncopyable { public: class Save { public: Save(); Save(DrawingContext &ct); ~Save(); void save(DrawingContext &ct); private: DrawingContext *_ct; };

and in drawing-context.cpp those methods all use ct and _ct for the variable names. Which is all fine and would be no issue at all, except that below the lines just cited in drawing-context.h every place one finds a reference to ct or _ct it is a cairo_t or a cairo_t*, not a DrawingContext.

Can somebody please explain why it is considered helpful in C++ to use "ct" to mean two different things like this in the one header file?

The disadvantage of doing it the way it is now is that when somebody like me encounters _ct elsewhere in src/display, and looks for it like this:

grep '[\* ]_ct[; {]' src/display/*.h

it finds two definitions:

  DrawingContext *_ct;

and

  cairo_t *_ct;

with the end result that one must read the file to figure out what is actually going on. Had the first set used "DC"/"_DC", for instance, there would have been no confusion, and the second step would have been eliminated.

I don't think this has anything to do with C/C++.

There is no name clash (the one is "DrawingContext::Save::_ct", the other "DrawingContext::_ct"), but it is confusing and I agree the former should be changed.

- Johan

On 22-1-2014 22:17, Martin Owens wrote:

On Wed, 2014-01-22 at 21:53 +0100, Johan Engelen wrote:

...

There is no name clash (the one is "DrawingContext::Save::_ct", the other "DrawingContext::_ct"), but it is confusing and I agree the former should be changed.

Since C++ is a type-safe language, is there any lint module that can find these errors? variable names with different types?

Not sure exactly what you mean. (To be clear, the code is not an error for the compiler.)

Some more explanation perhaps helps: Note that the Save class is not part of the DrawingContext class. It is merely defined in the namespace DrawingContext. There are two separate classes defined there: "DrawingContex::Save" and "DrawingContext". (ironically, DrawingContext is actually a part of the DrawingContext::Save class, as a member variable. So in the Save class, you could write "_ct->_ct", were it not that _ct is a private member of DrawingContext)

Scanning for duplicate variable names with different types in a file (or translation unit) is pointless, because it is bound to happen a lot for short variable names. Using grep, as David described, to find the declaration or definition of a variable name without looking at the context around the grep results is rarely a viable strategy I would think (especially if you search through an entire folder). You have to trace the scope hierarchy manually (e.g. function body -> class declaration -> base class declaration(s)), or use an editor that understands that.

Ciao, Johan

A little late to this party, but I'm OK with changing the variable name to "dc" if it was confusing.

2014/1/22 Johan Engelen <jbc.engelen@...2592...>:

(ironically, DrawingContext is actually a part of the DrawingContext::Save class, as a member variable. So in the Save class, you could write "_ct->_ct", were it not that _ct is a private member of DrawingContext)

Pedantic note: only a reference to DrawingContext is a member variable of Save.

Regards, Krzysztof