Your interface is missing a
virtual
destructor. This is an important thing for an abstract interface and your code is actually invoking undefined behavior when trying to destroy an instance ofIRandom
without one. With C++11, the proper way of declaring a virtual destructor in a pure interface class is using the= default
destructor idion:virtual ~IRandom() = default;
Using
double_t
is very unusual, sincedouble
is normally the same but with less typing. Unless you really have a good reason fordouble_t
, you should stick to the nativedouble
instead. I would say the same about usingint32_t
. Unless you really care about using strictly sized 32bit integers, just go for a plain ol'int
.By the way, when using the sized types, like
int32_t
and friends, from<cstdint>
, make sure to include the header file. You are relying on a residual include probably from<random>
, which might not be there on other compilers or even on a future version of your current one. At any rate, it is also good practice making dependencies clear.When using the types from
<cstdint>
, the correct way of referencing its members is thru thestd::
namespace. They happen to also visible at the global level because on most compilersstdint.h
(the C header) andcstdint
(the C++ header) are implemented as wrappers to the same file. There is no requirement from the C++ Standard that this be implemented is such way, so a C++ compiler is not required to provideint32_t
and friends outside thestd
namespace when includingcstdint
.std::numeric_limits<T>::[min][max]()
is preferred over theINT*_MAX / INT*_MIN
macros.Prefixing in-class member access with
this->
is very rare in C++. You should only be doing that when, for instance, you need to disambiguate some shadowed member access (but even that is discouraged because shadowing members is a bit of a design flaw; better to uniquely name your variables and methods).Identifiers beginning with underscore carry a few restrictions in C++ (Read this Read this). Your use inside a class for member variables is not wrong, but might be frowned upon. Consider perhaps other styles, such as the
m_prefix
or and underscoreatTheEnd_
. Or no distinction at all for member variables.I think you probably meant to camelCase
_bytedistribution
but mistyped thed
in "distribution".This is not the ideal way of initializing data in a constructor:
Your interface is missing a
virtual
destructor. This is an important thing for an abstract interface and your code is actually invoking undefined behavior when trying to destroy an instance ofIRandom
without one. With C++11, the proper way of declaring a virtual destructor in a pure interface class is using the= default
destructor idion:virtual ~IRandom() = default;
Using
double_t
is very unusual, sincedouble
is normally the same but with less typing. Unless you really have a good reason fordouble_t
, you should stick to the nativedouble
instead. I would say the same about usingint32_t
. Unless you really care about using strictly sized 32bit integers, just go for a plain ol'int
.By the way, when using the sized types, like
int32_t
and friends, from<cstdint>
, make sure to include the header file. You are relying on a residual include probably from<random>
, which might not be there on other compilers or even on a future version of your current one. At any rate, it is also good practice making dependencies clear.When using the types from
<cstdint>
, the correct way of referencing its members is thru thestd::
namespace. They happen to also visible at the global level because on most compilersstdint.h
(the C header) andcstdint
(the C++ header) are implemented as wrappers to the same file. There is no requirement from the C++ Standard that this be implemented is such way, so a C++ compiler is not required to provideint32_t
and friends outside thestd
namespace when includingcstdint
.std::numeric_limits<T>::[min][max]()
is preferred over theINT*_MAX / INT*_MIN
macros.Prefixing in-class member access with
this->
is very rare in C++. You should only be doing that when, for instance, you need to disambiguate some shadowed member access (but even that is discouraged because shadowing members is a bit of a design flaw; better to uniquely name your variables and methods).Identifiers beginning with underscore carry a few restrictions in C++ (Read this). Your use inside a class for member variables is not wrong, but might be frowned upon. Consider perhaps other styles, such as the
m_prefix
or and underscoreatTheEnd_
. Or no distinction at all for member variables.I think you probably meant to camelCase
_bytedistribution
but mistyped thed
in "distribution".This is not the ideal way of initializing data in a constructor:
Your interface is missing a
virtual
destructor. This is an important thing for an abstract interface and your code is actually invoking undefined behavior when trying to destroy an instance ofIRandom
without one. With C++11, the proper way of declaring a virtual destructor in a pure interface class is using the= default
destructor idion:virtual ~IRandom() = default;
Using
double_t
is very unusual, sincedouble
is normally the same but with less typing. Unless you really have a good reason fordouble_t
, you should stick to the nativedouble
instead. I would say the same about usingint32_t
. Unless you really care about using strictly sized 32bit integers, just go for a plain ol'int
.By the way, when using the sized types, like
int32_t
and friends, from<cstdint>
, make sure to include the header file. You are relying on a residual include probably from<random>
, which might not be there on other compilers or even on a future version of your current one. At any rate, it is also good practice making dependencies clear.When using the types from
<cstdint>
, the correct way of referencing its members is thru thestd::
namespace. They happen to also visible at the global level because on most compilersstdint.h
(the C header) andcstdint
(the C++ header) are implemented as wrappers to the same file. There is no requirement from the C++ Standard that this be implemented is such way, so a C++ compiler is not required to provideint32_t
and friends outside thestd
namespace when includingcstdint
.std::numeric_limits<T>::[min][max]()
is preferred over theINT*_MAX / INT*_MIN
macros.Prefixing in-class member access with
this->
is very rare in C++. You should only be doing that when, for instance, you need to disambiguate some shadowed member access (but even that is discouraged because shadowing members is a bit of a design flaw; better to uniquely name your variables and methods).Identifiers beginning with underscore carry a few restrictions in C++ (Read this). Your use inside a class for member variables is not wrong, but might be frowned upon. Consider perhaps other styles, such as the
m_prefix
or and underscoreatTheEnd_
. Or no distinction at all for member variables.I think you probably meant to camelCase
_bytedistribution
but mistyped thed
in "distribution".This is not the ideal way of initializing data in a constructor:
Note: Herb Sutter seems to disagree (thanks @ChrisDrew @ChrisDrew for the comment). Personally, I am down for auto
and type inference whenever it makes code shorter or easier to read. In this particular case, it is just more typing (which defeats one of the purposes of type inference, BTW). The constructor initialization syntax has been around since day 1, so it is pretty well rooted and known. The main argument for the auto var = Type()
style would be to make things more uniform, however, when declaring a member variable, the type still has to go on the left hand side, so the argument for consistency is only partial.
Note: Herb Sutter seems to disagree (thanks @ChrisDrew for the comment). Personally, I am down for auto
and type inference whenever it makes code shorter or easier to read. In this particular case, it is just more typing (which defeats one of the purposes of type inference, BTW). The constructor initialization syntax has been around since day 1, so it is pretty well rooted and known. The main argument for the auto var = Type()
style would be to make things more uniform, however, when declaring a member variable, the type still has to go on the left hand side, so the argument for consistency is only partial.
Note: Herb Sutter seems to disagree (thanks @ChrisDrew for the comment). Personally, I am down for auto
and type inference whenever it makes code shorter or easier to read. In this particular case, it is just more typing (which defeats one of the purposes of type inference, BTW). The constructor initialization syntax has been around since day 1, so it is pretty well rooted and known. The main argument for the auto var = Type()
style would be to make things more uniform, however, when declaring a member variable, the type still has to go on the left hand side, so the argument for consistency is only partial.
Note: Herb Sutter seems to disagree (thanks @ChrisDrew for the comment). Personally, I am down for auto
and type inference whenever it makes code shorter or easier to read. In this particular case, it is just more typing (which defeats one of the purposes of type inference, BTW). The constructor initialization syntax has been around since day 1, so it is pretty well rooted and known. The main argument for the auto var = Type()
style would be to make things more uniform, however, when declaring a member variable, the type still has to go on the left hand side, so the argument for consistency is only partial.
Note: Herb Sutter seems to disagree (thanks @ChrisDrew for the comment). Personally, I am down for auto
and type inference whenever it makes code shorter or easier to read. In this particular case, it is just more typing (which defeats one of the purposes of type inference, BTW). The constructor initialization syntax has been around since day 1, so it is pretty well rooted and known. The main argument for the auto var = Type()
style would be to make things more uniform, however, when declaring a member variable, the type still has to go on the left hand side, so the argument for consistency is only partial.