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
2 Answers 2
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.
-
\$\begingroup\$ The
Base
destructor needs to be defined anyway, so there's no reason to make that pure virtual. \$\endgroup\$Mooing Duck– Mooing Duck2020年08月03日 16:25:39 +00:00Commented Aug 3, 2020 at 16:25 -
\$\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– yyny2020年08月03日 20:12:56 +00:00Commented Aug 3, 2020 at 20:12 -
\$\begingroup\$ @yyny For this type of debugging tasks you should be using
std::cerr
which is not buffered. \$\endgroup\$Loki Astari– Loki Astari2020年08月03日 21:47:45 +00:00Commented Aug 3, 2020 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– programmer2021年09月05日 00:58:19 +00:00Commented Sep 5, 2021 at 0:58
-
\$\begingroup\$ @programmer: Not that will not work. The
Derived1()
object will be copied into aBase
object b. This cuts off all the bits that are fromDerived1
(you just copy theBase
portion of the object. Change to makeb
a reference.Base& b = Derived1(); b.do_something();
This is known as slicing. \$\endgroup\$Loki Astari– Loki Astari2021年09月05日 23:48:03 +00:00Commented Sep 5, 2021 at 23:48
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!!!
}
```
-
2\$\begingroup\$ This might just be a problem with the simplified example in the question. \$\endgroup\$Jörg W Mittag– Jörg W Mittag2020年08月03日 15:02:09 +00:00Commented Aug 3, 2020 at 15:02