Using "m_" prefix for member variables in C++ classes

I was thinking ‘c_’ for Class variable rather than ‘c’ for const. I guess the real question is do we want to impose a convention on static class variables or just leave them alone? If we do use a convention, should it be s_? I’m fine with that.

EDIT: Made a change in oct-inttypes.h to use this convention. See octave: 8d6bdd961494.

Oh, I misunderstood. Yeah, I’ve already used s_ in other places for static class variables, so if it’s OK with you, then let’s go with that.

You can omit the files in the libgui directory by editing the definition of INPUT in Doxyfile.in. I’m not sure how to eliminate just the generated ui files, but yes, we should skip those and all the qterminal files.

Definitely. The contributed code was good, but written by someone with a definite C slant. I made that change here: octave: 80373844f8b2.

Eliminating libgui/ directory and yy* variables leaves 610 to take care. Still a fairly large number.

vars.list (6.0 KB)

@jwe: Each of the graphics objects defined in graphics.in.h has a member variable go_name which holds the graphics_object name. Should this be changed to m_go_name? I think so, but this will require changing genprops.awk. There are so many weird corner cases I’m finding with the renaming.

Looking at the generated graphics.h file, I see that go_name is static (and maybe could also be const?) so maybe s_?

So many corner cases. I rename go_name to s_go_name in this changeset: octave: f16cfb2d7d1b.

Don’t despair. Some day everything will be perfect!

LOL

@jwe: One thing I am seeing in the generated documentation is a lot of member variables for graphics properties such as keypressfcn. In graphics.h this member variable is correctly labeled as m_keypressfcn, but Doxygen is also indexing graphics.in.h. That seems wrong to me, but maybe I don’t fully understand all of the use cases for Doxygen. For my own work I have temporarily excluded these files by adding

EXCLUDE_PATTERNS = *.in.h

Should I check in that change?

I think we should tell doxygen to only look at the files that are compiled, so yes, we should skip any *.in.* files. But we should also tell it how to find the generated sources. Are you building in the source tree? If the build is outside of the source tree, I don’t see how doxygen will find the generated graphics.h file since I don’t see the build directories listed in the doxygen configuration file.

Yes, I am.

EDIT: Also, INPUT is defined using @abs_top_srcdir@ so regardless of where compilation is done, Doxygen is going to find the source code including graphics.h and graphics.in.h.

EDIT: I excluded *.in.h files from Doxygen generation in this cset: octave: 13c04ecbfbe1.

1 Like

I will take car of the files in libgui/src. Some of the GUI classes still use the _.... notation for member variables.

If building in a directory outside of the source tree, then I expect it will not find the files that are generated in the build directories.

But we want to find the generated file graphics.h and not the raw input file graphics.in.h. In this case, it seems better to build in-tree so I can capture that file.

@jwe: I’m really happy that this naming convention is being implemented. Previously I would see a variable being assigned a value and I would have to scroll up in the code to see if it was a local variable to the function. If I found a declaration then it was definitely a local variable. If I didn’t find it, I would generally assume that it was probably a member variable. All of this was rather tedious when trying to understand what a piece of code was doing. Now it is quite clear which variables are member variables, which are function local variables, and which are static class variables.

Should have done this sooner I guess.

@jwe: Clearly coders were working around having many symbols (functions, variables, etc.) with the same name. Unfortunately they seemed to have chosen all sorts of workarounds. Now that we are trying for a consistent standard (“m_”), is it okay/worthwhile to rename some of these member variables?

As an example, consider sparse-lu.h which defines Lfact and Ufact as member variables with getter functions named L and U. It would make more sense to me if the member variable was named m_L and the getter function was changed from

      lu_type L (void) const { return m_Lfact; }

to

      lu_type L (void) const { return m_L; }

If we did do these internal renamings I think there is no change to the API. Programmers were meant to use the getter routines to access the internal member variables which were declared private.

EDIT: I renamed sparse-lu.h member variables in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/93f576a35b23) and lu.h variables in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/43622407af81).

@jwe: In the hess class there are member variables hess_mat and unitary_hess_mat. In the schur class there are member variables schur_mat and unitary_mat. For consistency, shouldn’t this be unitary_schur_mat?

EDIT: I renamed unitary_mat in this changeset (octave: 1e277c6b6626).

@rik:

That seems like an OK change.

For base classes like base_diff_eqn declared in base-dae.h in which the members are protected, how cautious to we want to be with these changes? Are there derived classes outside of Octave that might use the previous names directly? How important is it for us to provide backward compatibility? I’d say “low” as I think these are fairly obscure classes.

I agree that the probability of someone having derived classes off of these is is pretty low. Given the extra work to include deprecated code for two Octave versions I would just implement this and see if we get any complaints.