6
\$\begingroup\$

I started learning programing with C++. It runs perfectly, but I wonder what things I can improve following the principles of class inheritance, access, overwriting, virtual functions and destructors, and why.

#include <iostream>  // allows program to output data to the screen
using namespace std;


struct Base {
    virtual void do_something() {};
    virtual ~Base(){};

};

struct Derived1 : public Base {
    Derived1():Base(){}
    virtual void do_something() {
        std::cout << "Derived1!!!" << std::endl;
    }
    virtual~Derived1(){};
};

struct Derived2 : public Base {
   Derived2():Base(){}
    virtual void do_something() {
        std::cout << "Derived2!!!" << std::endl;
    }
    virtual ~Derived2(){};
};

// function main begins program execution
int main(int argc, const char *argv[]) {
    std::cout << "Welcome" << std::endl;

    Derived1 derived1;
    derived1.do_something();

    Derived2 derived2;
    derived2.do_something();

    return 0;

}  // end function main

\$\endgroup\$
13
\$\begingroup\$

Don't add meaningless comments.

#include <iostream>  // allows program to output data to the screen

There is a real issue with code and comment rot. So your comments should always be meaningful as you have to maintain them with the code. It is best to reserve comments to "WHY" you are doing something. The code will explain "HOW".


Don't do this:

using namespace std;

There are definitely issues with pulling the whole standard namespace into the global namespace.

See: Why is “using namespace std;” considered bad practice? In my opinion the best answer is the second one: sbi though the first one is good.


If Base does no real work you can make the virtual functions abstract:

struct Base {
    virtual void do_something() {};
    virtual ~Base(){};
};

// If the user should not be able to instantiate a `Base` then do this:

struct Base {
    virtual void do_something() = 0;
    virtual ~Base() {}
};

If your functions do not alter the standard behavior then don't include them:

struct Derived1 : public Base {
    Derived1():Base(){}
    virtual void do_something() {
        std::cout << "Derived1!!!" << std::endl;
    }
    virtual~Derived1(){};
};

Here the constructor and destructor are useless. Do not bother to specify them

struct Derived1 : public Base {
    virtual void do_something() {
        std::cout << "Derived1!!!" << std::endl;
    }
};

Don't use std::endl.

        std::cout << "Derived2!!!" << std::endl;

This is the major cause of C++ code running slowly. The problem is that std::endl forces the stream to flush. The stream will automatically flush when it is need and any extra flushes are likely to be inefficient (humans are bad at working out when to flush the stream).

It is better simply to use "\n"

        std::cout << "Derived2!!!" << "\n";

From C++11 we introduced the override specifier.
In the derived class you should mark any overridden methods with it.

struct Derived1 : public Base {
    virtual void do_something() override;
};

struct Derived2 : public Base {
    virtual void do_something() override;
};

The advantage here is that if in the future somebody changes the Base class and renames or alters the virtual functions in the base the compiler will not warn you that these functions no longer align with the base class version.

\$\endgroup\$
6
  • \$\begingroup\$ The Base destructor needs to be defined anyway, so there's no reason to make that pure virtual. \$\endgroup\$ Aug 3 '20 at 16:25
  • \$\begingroup\$ @MooingDuck Thanks Fixed. \$\endgroup\$ Aug 3 '20 at 17:23
  • \$\begingroup\$ std::endl is useful for debugging when code may prematurely crash or exit or when running code in multiple threads. For small log messages like this, std::endl is fine. For larger pieces of data, not so much. \$\endgroup\$
    – yyny
    Aug 3 '20 at 20:12
  • \$\begingroup\$ @yyny For this type of debugging tasks you should be using std::cerr which is not buffered. \$\endgroup\$ Aug 3 '20 at 21:47
  • \$\begingroup\$ Is it possible to do this: Base b = Derived1(); b.do_something(); ? For me it's not executing the do_something method of Derived, rather its running the base's method. Is there a way I can achieve b to execute Derived1's do_something method? \$\endgroup\$
    – programmer
    Sep 5 at 0:58
5
\$\begingroup\$

While Martin explained problems with code there is, I'm gonna note about one that isn't. This is done out of suspicion about you not knowing and I hope you'll forgive me if I'm wrong.

Currently you don't use virtuality of your virtual functions.

In the main function you create objects of derived type and call functions of derived type. You never use the API of base class

The use mostly arrives when

a) There's a container with base class pointers

std::vector<unique_ptr<Base>> vec;
vec.push_back(make_unique<Derived1>());
vec[0].do_something() //Derived1!!!

or

b) There's a function taking base class pointer/reference

void foo(Base& b)
{
   b.do_something();
}

int main()
{
  Derived1 d;
  foo(d); //Derived1!!!
}

Sometimes both:

void foo(Base& b)
{
   b.do_something();
}

int main()
{
  std::vector<unique_ptr<Base>> vec;
  vec.push_back(make_unique<Derived2>());
  foo(vec[0]); //Derived2!!!
} 
```
\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.