Posted inExperience / Information Technology

C++ — Never Do’s

seasoned_geek by seasoned_geek

No, Never Do isn’t a new loop type though it can be what happens when you ASS-U-ME a default value for a loop variable. These are some things you should never do in your code. I’ve spent nearly 40 years in IT now. For the past decade I’ve been working in the medical device realm. Prior to that I wrote great big boring systems for great big companies. This be what you don’t do in C++ today.

Never Override && or ||

The standard will let you do it, but it will never work correctly.

All of you know that if the first test fails the second test doesn’t even happen, correct? Not when you override &&. That only happens for the built-in operator. When you declare the && operator for your class, you cannot even be sure which expression will be evaluated first. Think that is a minor inefficiency?

MyBadClass abc;
MyBadClass def;
MyBadClass ghi;
MyBadClass jkl;

...

if ( (abc || def) && (ghi || jkl) )

What happens if you’re custom class that overrode these did it because you thought it would be way cool to both step the motor in the robot arm while testing? The last two movements aren’t supposed to happen until the first two have cut as far as needed in a human heart.

Never Omit {} Around a Single Statement Condition

You will actually find that one in the Barr Group Coding Standard for Embedded Systems. It’s a good rule. One I arrived at after thinking code “looked better” without them. When you do this

if (!eof)
   idx.read_next();

is you end up needing some kind of debug statement or additional logic. While it may seem trivial in a 2-line example when you’ve got a screen length if-else-if-else nested to various levels and most having just one line, it’s a house of cards waiting for someone to kick the table. 99 times out of a hundred it’s not even properly aligned.

I’ve opened up source code that had over a hundred line if statement that looked much like the above only they also indented when the true side of the if had another if.

Never Put { on the Same Line as Code

No, that’s not valid C++. That’s for Java and JavaScript. Again this is one from the Barr Coding Standard and it is a good one. Lots of studies have been done and it is determined that the human brain needs the visual separation. We need to visually know with just a glance what language we are looking at. There have been too many C++ wanna-be languages and now we are in multi-language projects.

Know this, the embedded systems will be in C/C++ because you cannot use Python for a medical device. FDA regulations mandate static source code analysis on a type-safe language. Sometimes you get forced into situations like this and the boundaries matter.

void MainWindow::menuShortCutsFromScintilla()
{
    EdtEditWidget *ptr = static_cast<EdtEditWidget *>( m_tabWidget->currentWidget() );
    QString keySequence;

    if ( ptr )
    {
        Scintilla::Message msg = static_cast<Scintilla::Message>( SCI_REDO );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionRedo->setShortcut( QKeySequence( keySequence ) );
        }

        msg = static_cast<Scintilla::Message>( SCI_UNDO );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionUndo->setShortcut( QKeySequence( keySequence ) );
        }

        msg = static_cast<Scintilla::Message>( SCI_CUT );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionCut->setShortcut( QKeySequence( keySequence ) );
        }

        msg = static_cast<Scintilla::Message>( SCI_COPY );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionCopy->setShortcut( QKeySequence( keySequence ) );
        }

        msg = static_cast<Scintilla::Message>( SCI_PASTE );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionPaste->setShortcut( QKeySequence( keySequence ) );
        }

        msg = static_cast<Scintilla::Message>( SCI_SELECTALL );
        keySequence = ptr->editor()->keyForMessage( msg, 1 );

        if ( keySequence.length() > 0 )
        {
            m_actionSelectAll->setShortcut( QKeySequence( keySequence ) );
        }
    }

    keySequence = Overlord::getInstance()->beginSelectionKeySequence();

    if ( keySequence.length() > 0 )
    {
        m_actionSelectionBegin->setShortcut( QKeySequence( keySequence ) );
    }

    keySequence = Overlord::getInstance()->clearSelectionKeySequence();

    if ( keySequence.length() > 0 )
    {
        m_actionSelectionClear->setShortcut( QKeySequence( keySequence ) );
    }
}

If you think that is unreasonable, think again. That’s from a programmer’s text editor that allows one to override default keyboard shortcuts.

Never Put More Than 130 Characters on a Line

Today’s text editors will let you go to unbelievable lengths on a line but it is impossible for programmers to “see” the logic flow when you do that. The Barr standard actually wants less than 80. I understand, but with C++ that isn’t realistic. It’s not just because terminals used to support only 80 columns. Lots of code had to be sent via UseNet or to paper terminals which doubled as printers. We also had IBM TCAM to deal with. Because of the additional control codes (<CR><LF><STX><ETX> and a couple others) you could only send 72 printable characters without wrapping.

Today, in a nice readable font, you can easily display 130 characters. Printers are more than capable of printing 132 characters on a line so nothing wraps when you need to make a printout. Most importantly, if you limit the lines to one screen height per function, a developer can see the flow.

When you open a source file and see a bunch of lines running off the right side of the screen you know that was written by a hack!

Never Bury a Private Class in a Source File

When you see stuff like this

know that you are dealing with an archaic legacy application/framework. Typically they tried to span languages between C++ and non-type-safe languages like Python, JavaScript, insert-scripting-language-here. The only way they could make that house of cards function was to bury the functionality. If your use case doesn’t fit into the original developer’s tiny mind, you can’t make it work. You have to pull in a ton of their build configuration and dependencies just to override a simple thing. You can read a bit more about this in my Death of Commercial Application Frameworks post.

None of you write tight enough code to bury it in a hidden class. None of you can make proper decisions for all use cases when it comes to what to make private. If your use case wants/needs a buried “private” class you can simply inherit private.

class MyBetterClass : private MyBaseClass
{
...
};

For your use class everything from MyBaseClass will be private in your derived class. If you want almost all of it to be private you can override certain methods and/or member variables. If they were public in the base class and you want them public (or protected) in your derived class.

class MyBetterClass : private MyBaseClass
{
public:
   using MyBaseClass::nukeSomething();
   
 protected:
    using MyBaseClass::visible;

Just remember, you can increase protection when deriving, you cannot decrease it.

Single Exception to This Never

Human/animal safety is the one time it is recommended to bury access in a hidden private class.

When you are developing a custom carrier board for a SoM or SoC one can override/re-purpose a great many of the edge connector pins as well as the expansion and header pins. Any time one of those pins is used for human/animal safety it is advisable to bury all access to it in a hidden private class.

What kind of safety?

  • If this pin has voltage the robot arm is physically blocked from starting surgery because staff haven’t completed one or many pre-surgical safety tasks.
  • Until this pin gets voltage the forklift cannot move. The pin won’t get voltage until a sensor stating the operator’s ass is in the seat sends voltage in on some other pin.
  • The automobile cannot be put in gear until this pin has power and this pin will not have power until the drunk has blown into the breathalyzer and it determines they are legal to drive.

You get the idea. Those pins have to be protected so they cannot be randomly written to via stray pointer or some other code anomaly.

Never Provide Comparison Operators Without Explicit Constructors

This fries a lot of eggs. Let’s go back to the C++ Initialization and Constructors Post.

class FileDetails
{
public:

    FileDetails( std::optional<const QString> fileName      = std::nullopt,
                 std::optional<const QString> cacheFileName = std::nullopt,
                 std::optional<const QString> lexerName     = std::nullopt,
                 std::optional<const QString> tabText       = std::nullopt,
                 std::optional<int>           eolMode       = std::nullopt,
                 std::optional<sptr_t>        pos           = std::nullopt,
                 std::optional<sptr_t>        lineNumber    = std::nullopt,
                 std::optional<int>           lineWrap      = std::nullopt,
                 std::optional<bool>          readOnly      = std::nullopt,
                 std::optional<bool>          autoIndent    = std::nullopt,
                 std::optional<bool>          isDirty       = std::nullopt );

    FileDetails( const FileDetails &other ) = default;
    
    QString fileName();

...

private:
    // QString initializes itself to empty string
    // only have to set the one we need filled in
    //
    QString m_fileName;
    QString m_cacheFileName;
    QString m_lexerName {"null"};
    QString m_tabText;
    int     m_eolMode {SC_EOL_LF};
    sptr_t  m_pos {0};
    sptr_t  m_lineNumber {0};
    int     m_lineWrap {SC_WRAP_NONE};
    bool    m_readOnly {false};
    bool    m_isDirty  {false};
    bool    m_autoIndentEnabled {false};
};

That constructor would have to have an explicit clause on it if I were to provide ==, <, or > operators. This happens a lot with classes that have numeric parameters but I can show you here as well. Let us assume such operators are defined, what happens when someone gets lazy, tired, or distracted while coding. That want to see if the value in m_fileName matches a given name.

FileDetails fred("instructions.txt");

...

// Oops! This constructs a FileDetails object and compares them
// names are same but rest of fields will not be and will return false.
if (fred == "instructions.txt")

...

// Oops! This constructs a FileDetails object with main.cpp and assigns it to fred
// should always return true.
if (fred = "main.cpp")

While some of today’s compilers will catch the use of a single = in the second if, Yoda Coding allows even old compilers to catch it.

if ("main.cpp" = fred)

Will fail to compile every time.

What the programmer needed to do was use fred.fileName(), but they got in a hurry. Because this class needed to be easy to use it did not put explicit on the constructor. If it had one would need to do this:

FileDetails Ethyl( QString("MainWindow.cpp"));

because it couldn’t promote an ordinary string to a QString.

Never Put Locals Before Globals

How many of you know that #include “myclass.h” is a local include and #include <stdio.h> is a global include? During the days of DOS, 20 MEG hard drives and 1.2 MEG floppy disks this was a much needed optimization. I/O was sloooooowwwww. You only wanted to look one place for a file.

C and even C++ programmers got enamored with MACROS. I worked in one shop where a single developer had tried to write the entire system using macros. Early compilers wouldn’t flag macro re-definition as any kind of warning or error. They are a bit better today, but even Microsoft got burned by this recently.

Honestly, I’ve tried to use libraries from different places and had one do this:

#define TRUE 1
#define FALSE 0

and another do this:

#define TRUE 0
#define FALSE 1

nice, huh? You can still torpedo yourself from afar by putting your own local include files ahead of global include files in source files.

#include "MyInclude.h"
#include <thirdpartylib.h>    // some third party library main header file

Now, let us assume somewhere inside MyInclude.h you have the following:

#define EXTENSION txt

But in thirdpartylib.h you have

#define SUPPORTED_FILE_EXTENSION_LIST ".cpp,.c,.h,.hpp,.cxx,.hxx"

The library you link against was compiled with that definition of the macro. Trouble is, after processing your local include the compiler will change the macro to be

#define SUPPORTED_FILE_txt_LIST ".cpp,.c,.h,.hpp,.cxx,.hxx"

Yes, compilers have gotten better about catching such things. You can minimize self-inflicted damage by placing your locals after globals. Most IDEs with code generation make this difficult though.

Never Use Smart Pointers in a Mixed Language Application

Your C++ compilation unit doesn’t know about the outside world. You don’t know about the outside world. All you know is you are calling some OS provided function or third party library. It has no idea bout your Smart Pointer. For more details on the subject, read this post.

Roland Hughes started his IT career in the early 1980s. He quickly became a consultant and president of Logikal Solutions, a software consulting firm specializing in OpenVMS application and C++/Qt touchscreen/embedded Linux development. Early in his career he became involved in what is now called cross platform development. Given the dearth of useful books on the subject he ventured into the world of professional author in 1995 writing the first of the "Zinc It!" book series for John Gordon Burke Publisher, Inc.

A decade later he released a massive (nearly 800 pages) tome "The Minimum You Need to Know to Be an OpenVMS Application Developer" which tried to encapsulate the essential skills gained over what was nearly a 20 year career at that point. From there "The Minimum You Need to Know" book series was born.

Three years later he wrote his first novel "Infinite Exposure" which got much notice from people involved in the banking and financial security worlds. Some of the attacks predicted in that book have since come to pass. While it was not originally intended to be a trilogy, it became the first book of "The Earth That Was" trilogy:
Infinite Exposure
Lesedi - The Greatest Lie Ever Told
John Smith - Last Known Survivor of the Microsoft Wars

When he is not consulting Roland Hughes posts about technology and sometimes politics on his blog. He also has regularly scheduled Sunday posts appearing on the Interesting Authors blog.