Code Readability/Organization conventions versus standards

[primem0ver]

This is a question I thought I would throw out there because lately I have been pondering using custom conventions to make code more readable and organized versus using standard coding conventions when defining a class.

It might seem a bit nitpicky or extreme to some, but I prefer to organize items in a class by category (fields, methods, overrides, inlines, etc...) rather than mixing them all up under a single access keyword (private, protected, public).

Is it a bad idea to use a custom define convention to accomplish this if I plan on sharing the code with a team or publishing the library? Would people appreciate the organization or would the reliance on non-standard directives be annoying/problematic?

Here are examples of defines used in place of access keywords:

#define methods public
#define restricted_methods protected
#define overrides public 
#define restricted_overrides protected
#define fields private
#define common_fields protected 

[mbozzi]

Is it a bad idea

Yes, certainly.

Consider:

| New scheme                               | Conventional scheme           |
|------------------------------------------+-------------------------------|
| methods: void f();                       | public: void f();             |
| restricted_methods: void f();            | protected: void f();          |
| overrides: void f() override;            | public: void f() override;    |
| restricted_overrides: void f() override; | protected: void f() override; |
| fields: int x;                           | private: int x;               |
| common_fields: int x;                    | protected: int x;             |

Literally every C++ developer understands public, protected, and private. Nobody understands these macros, and you have to keep undef'ing them at the end of your public headers. The macros are not clearer at all if one simply learns what the keywords mean.

Do not make it unnecessarily difficult to read your code. The organizational pattern itself is probably fine where it can be applied, but explain the convention in the documentation and follow it in the code. Without the macros.

In any event, the names should be in ALL_CAPS so that it's clear they are macros.

[JLBorges]

> I prefer to organize items in a class by category (fields, methods, overrides, inlines, etc...)
> rather than mixing them all up under a single access keyword (private, protected, public).
> Is it a bad idea to use a custom define convention to accomplish this

You can use comments or custom attributes to accomplish the same goal.
(Unknown attributes do not result in an error; they are ignored by the compiler).

struct A
{
    public: // *** methods ***

        [[pm0::method]] void some_function() ;
        // ...

        
    public: // *** overrides ***
        // ...

        
    protected: // *** restricted_methods ***

        [[pm0::restricted_method]] void some_shared_implementation() ;
        // ...

        
    protected: // *** restricted_overrides ***

        // ...

        
    protected: // *** common_fields ***

        // ...

        
    private: // *** fields ***

        // ...
};

[primem0ver]

Alright. Thanks for the feedback; both of you. What about blank macros that went after each access keyword? Such as

#define FIELDS 
// or
#define METHODS: /* Methods */

class A
{
protected FIELDS:
...
public METHODS:
}; 

]

EDIT: Just as some background... I used to use comments (which are a pain to have to write constantly). I considered this convention after working with Qt. They have some custom keywords such as "signals". However, when I went looking for how they were recognized, they appeared to be blank macros. So I copied their convention using something similar to the code above (though with lower case letters).

[primem0ver]

mbozzi wrote:

The organizational pattern itself is probably fine where it can be applied, but explain the convention in the documentation and follow it in the code. Without the macros.

Can you explain this? Are you suggesting I should make custom keywords? I am not sure how that would be different (unless they don't need to be undefined).

[keskiverto]

Qt. They have some custom keywords such as "signals".

Qt has an additional preprocessor that parses the headers and generates additional code. Those "custom keywords" are for that tool, not for compiler.


Which is more a pain to write:

protected FIELDS:

protected: // fields 

Don't you have an editor that could create a template for class definition with commented access specifiers? (Preferably you give just name and it creates header file with guards, boiler plate, copyright statement, etc.) I knew a dev that had one ... two decades ago.

[Repeater]

As keskiverto and others say, this can be achieved with something C++ already supports and that every C++ programmer already understands; comments.

If you want to organise your functions into groupings depending on whether they're override or private variables or whatever, knock yourself out and put a little comment at the top of each group saying what they are.

Would people appreciate the organization or would the reliance on non-standard directives be annoying/problematic?

I make you this solemn promise. They will hate it. They will despise it. I've seen people use the preprocessor to create their own language based on C++ like this, and it's not popular. Especially when they then have to constantly keep reminding people to use it. The other programmers haven't forgotten; they're deliberately not using it.

[jonnin]

this reminds me of people who made macros to make C look like pascal.
if you want to write in pascal, use pascal. If you want to write in C++, use C++.
It absolutely is NEVER more readable to rename keywords to something obscure. It is always confusing to people who know the language and creates more problems than it solves. Its really just trying to justify 'I don't want to learn the language' or 'I don't like how this looks'.

[doug4]

The other programmers haven't forgotten; they're deliberately not using it.

At another company I used to work for, there was a department in Europe where one guy was spearheading a major project. He was championing a major state machine tool (I think he had been involved in some research around the tool), and everything about the project was structured to use the state machines whenever possible. The code was slow, cumbersome, and very spaghetti-like. A single external event would get processed and fed back in as internal events over and over again. Simple control logic (if/else, switch) was processed using feedback events rather than C++ control statements (because that's how you were supposed to do it in a state machine framework).

The guy took a 6 or 7 week vacation (this was Europe after all), and while he was away, the rest of the team rewrote the entire project in straight C++ without the state machine infrastructure. The code was immediately faster and easier to understand, and the rest of the team loved it. When he got back from vacation, he saw what had been done, and promptly resigned.

There are a number of lessons to learn here. One is, don't force other to use constructs that are cumbersome and obtuse and don't add any benefit. They won't unless forced to do so.


p.s. By the way, I love state machine programming. I worked on a related project using the same tool. The state machine itself was probably originally designed by the same people, and every state in the system was represented by an input in the state machine, and every input in the system was represented by a state in the state machine. It drove me batty. Please don't take this as a denigration of state machines in general, and not even of the specific tool being used--just the specific state machine insances used in these particular projects.

[Jaybob66]

jonnin wrote:

this reminds me of people who made macros to make C look like pascal.

me too, this caused the biggest row of all time, a graduate tried to get it added to the coding standards and QA didnt dismiss it quick enough for our liking :)

#define BEGIN {
#define END } 

[Thomas1965]

(Unknown attributes do not result in an error; they are ignored by the compiler).

Why does it create then a warning(C5030) VS 2017 ?

[keskiverto]

A warning isn't an error. A compiler that says nothing about "odd stuff" is less useful than Verbal Kint. One can usually set the level of verbosity of the compiler.

[primem0ver]

LOL. Allright then. Thanks for the feedback.

[mbozzi]

Can you explain this? Are you suggesting I should make custom keywords? I am not sure how that would be different (unless they don't need to be undefined).

I'm trying to suggest explaining the grouping you expect in the documentation, and leaving the custom keywords out.

For example, I sometimes prefer to organize my classes in public/protected/private order.

I've never bothered to enforce this, but if I wanted to, I'd just put a note in the CONTRIBUTING file or the style guide that says "Where possible, class member specifications shall group member declarations by access specifier, in public/protected/private order", or something like that. You could do something similar ("... methods/overrides/fields order").

But, if this isn't sufficient, then I like JLBorges's suggestion of a special comment or custom attribute. This would allow the guideline to be enforced by a machine as part of the build.

[Jaybob66]

i always group members by feature

class myclass
{
   ///////////////////////////////
   // initialisation

public:
   myclass();
   myclass(int i);

protected:
   int populate(int i);
   
   ////////////////////////////////
   // data manip

public:
   bool set(int someData);
   int get();
protected:
  int localData;

};

i never really understood OO developers who organised by anything other than purpose, just seems to go against the grain for me.

[Repeater]

Whereas if I was looking at that to see what functions and variables the class provided to me, and then I realised that I couldn't see all the public functions in one place but would have to scroll up and down looking in different places for them, that would go massively against the grain for me.

[keskiverto]

One should not underestimate the comments either. With special comment formatting a tool like doxygen can generate/maintain more than barebones hypertext documentation for each member. Doxygen won't do that from (empty) macros, will it?

[MikeyBoy]

Jaybob66 wrote:

i always group members by feature

That seems like a code smell to me. If a single class has so many members, dealing with so many different features, then it suggests to me that you need to break that class into smaller classes, with better-defined responsibilities.

Repeater wrote:

Whereas if I was looking at that to see what functions and variables the class provided to me, and then I realised that I couldn't see all the public functions in one place but would have to scroll up and down looking in different places for them, that would go massively against the grain for me.

Indeed. If I'm looking at a header file, it's most likely to be because I want to understand how to use the class. Therefore, the important part of the class definition is the public part. That's the part I'll be using, so that's the part I need to be able to see and understand easily.

If the person writing the code has obscured the public interface by splitting it up into several sections, and mixing those up with private implementation sections, then that is, at best, something that impedes my ability to use their class, and at worst, a compelling reason to get rid of that class, and replace it with one written by someone who actually gives a damn about their users.