Ordering of public/private sections in C++ class

While updating code to use “m_” for class member variables I came across this class in `oct-norm.c``

  // norm accumulator for the p-norm
  template <typename R>
  class norm_accumulator_p
  {
    R p,scl,sum;
  public:
    norm_accumulator_p () { } // we need this one for Array
    norm_accumulator_p (R pp) : p(pp), scl(0), sum(1) { }

    template <typename U>
    void accum (U val)
    {
      octave_quit ();
      R t = std::abs (val);
      if (scl == t) // we need this to handle Infs properly
        sum += 1;
      else if (scl < t)
        {
          sum *= std::pow (scl/t, p);
          sum += 1;
          scl = t;
        }
      else if (t != 0)
        sum += std::pow (t/scl, p);
    }
    operator R () { return scl * std::pow (sum, 1/p); }
  };

Unlike most Octave code, it does not use the keyword private: to designate the variables that are, in fact, private to the class. Instead it relies on the C++ standard which says all members of a class are private unless specifically made either protected or public.

While the above is all true, I find it useful to specifically mark which sections of a class are private. In effect, it is like extra documentation and I don’t need to think so hard about whether the object has been defined as a class, struct, or union and what the defaults are for each possibility.

If others agree, I think we should add a recommendation to the C++ style guide (C++ style guide - Octave) to always use the access specifier for classes.

The second observation is that most Octave C++ classes define the public interface first, and then the private implementation second. I find this useful because if I am going to use a class I need to see what the constructor is first and then what the publicly available member functions are. If others agree, I think we should add another recommendation to the C++ style guide to order the sections of a class as 1) public, 2) protected, 3) private.

Yes, I agree with both.

I also like to have any functions in a source file appear in the same order as they are declared in the header file. Unfortunately, sometimes this conflicts with the public, protected, private ordering when a public function relies on a private helper function. In that case, I sometimes want to group the helper function along with the public function in the source file, just because they are related, but that doesn’t always match with the order of declarations in the header file.

@jwe: What is your opinion on the ordering of member functions and member variables within a class section?

For public: sections we very often start with member functions—specifically the class constructors and destructor. That makes a lot of sense to me.

For private: sections, however, I often see that we list the member variables before any private member functions. For example,

private:
  int var1;
  bool var2;

  void init ();

It makes more sense to me to continue to list the member functions first. Is it worthwhile making a recommendation about variable/function ordering as well?

EDIT: I added recommendations for ordering of access blocks and the ordering of member functions and variables within access blocks here: C++ style guide - Octave.

It’s fine with me to uniformly list variables last in class declarations. Maybe we could also introduce some kind of comment separator that makes it easy to spot the beginning of the variables section of the class declaration?

That’s a good idea. Any suggestions?

I’m not sure. I generally don’t like to see comments about apparently obvious things (I once worked on a project where the style was to write things like

C RETURN TO CALLING SUBROUTINE
      RETURN
      END

and I found that to be quite annoying).

But in this case, I think it might be helpful to have a marker that separates the function declarations from the variables. I’ve sometimes had to stop and search to distinguish them from the function declarations. I know it is possible that the comments could be incorrect and misleading if some variable declarations are not where they should be, but maybe just using something simple like // Member variables: would be helpful?

Like being a great prose writer, being a great coder requires striking a balance between conciseness and clarity. Go overboard on the first and you end up with obfuscated C contest entries. Go overboard on the second and you end up with pages and pages of code with obvious documentation like “This is the variable to hold an integer”. I dislike both extremes.

Since this is just a visual element to help break up sections I wonder whether it might be better not to use words (// Member variables:). This is a comment that requires reading and all I really need to know is that a section break has occurred. Would something like //----- be concise and obvious?

1 Like

A simple marker like you suggest is fine with me.

1 Like