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

At the Octave Developers Meeting today it was confirmed that we would like to use “m_” as a prefix for member variables in C++ classes. There had been no mention of this convention previously so I added it to the list of C++ Octave Coding Guidelines here: C++ style guide - Octave.

There are still a large number of variables (1092) which need renaming. Because the number is so large I’d like to just put the list out and when people have time they can convert a file or two at a time. With enough programmers this shouldn’t be too hard.

The easiest way I found to get a list of the member variables for a class was to run make doxyhtml at the top of a development tree. This will create files in the directory doc/doxyhtml. The files function_vars_[a-z].html contain the member variables that begin with each letter.

For reference, the current list of variables that need changing (8/24/21) is uploaded here.member_vars.list (13.8 KB)

There may be a large exclusion of variables related to Qt files generated by the ui processor. If someone could confirm that these do not need changing then the list should shorten considerably.

1 Like

@jwe: Just tried to resolve an instance of a variable name without an “m_” prefix. Code is in cquad.cc

// Data of a single interval.
typedef struct
{
  double a, b;
  double c[64];
  double fx[33];
  double igral, err;
  int depth, rdepth, ndiv;
} cquad_ival;

Should we add “m_” to straight structs? Or only structs that are actually classes? I’m favoring the latter.

@jwe: Should there be a convention for class variables as there is for member variables? For example, in oct-inttypes.h there is

   // The following are provided for convenience.
  913   static const octave_int zero, one;

Is a prefix like “c_” good because it adds clarity that these are class variables? Or is it just a distraction?

First success, I modified the chol class in liboctave/numeric. See octave: e156fce82e00.

1 Like

Interestingly, there seems to have been a recognition that one needed a way to distinguish member variables from, say, a getter function with the same name. It seems that x or x_ was a popular choice. I just modified mk-opts.pl to change the prefix generated for DASPK_options, DASRT_options, DASSL_options, LSODE_options, and Quad_options to use m_. This was a rich vein of changes. See octave: d8ae55260760.

1 Like

Yes, and you’ll also see places where a leading underscore is used or the argument names in the constructor were mangled so as not to conflict with the member variable names. Overall though, I prefer the m_ prefix.

@@ -50,7 +50,7 @@
 namespace octave
 {
   static Matrix
-  chol2inv_internal (const Matrix& r, bool is_upper = true)
+  chol2inv_internal (const Matrix& r, bool m_is_upper = true)
   {
     Matrix retval;

I wouldn’t have made that change because in this context is_upper is just a parameter to the static function, not an actual member variable.

I have used s_ prefix for a static class variable in other cases. I don’t think I would incorporate the constness of a symbol in the name itself.

Just finished ov-fcn.h and found both formulations. There was xdispatch_name and xpackage_name as well as my_name and my_dir_name. Using a single standard of “m_” makes a lot of sense as there was definitely a diversity of mangling conventions.

Thanks for the code review. I used Perl with a regexp for the search/replace operation so it caught all m/\bis_upper\b/ patterns. Fast, but slightly error prone. I will go change that.

EDIT: Reversed that change here: octave: 9fa1d8dd3a23.

Using typedef struct { ... } symbol_name is more C than C++. In C++ we can just write

struct cquad_ival
{
   ...
};

and since struct is just the same as a class with all public members it might seem like we should use m_ but there are no member functions here, so direct access to the members is clearly what is intended and I think I’d rather see foo.fx than foo.m_fx. But if we made the members private and provided accessor functions, then I’d probably want to use m_ for the member variables. For an internal struct, it’s probably best to leave it alone, except perhaps to change the definition to be more C++.

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