Fri 10 Oct 2008 02:54:55 PM UTC, comment #2:
Petr,
When you add code for new feature
I think you should try to use
- std::string instead of char*
unless you have good reason not to do so
- STL datatype instead of plain C-style array
... unless you have good reason not to do so
in this case the:
TransportType getTransportationHandle(const char *theName);
const char *getTransportationName(TransportType theType);
would be
TransportType getTransportationHandle(const std::string theName);
const std::string getTransportationName(TransportType theType);
std::string is OK here because it is an CERTI internal API.
struct TransportTypeList {
const char *name;
TransportType type;
};
is would typically be a [static] map<TransportType,std::string>
which may be initialized in ObjectManagement constructor.
May be in this case it is overkill because map
would have 2 entries but this may be easier to read and
maintain.
A last remark is regarding encapsulation.
Try to use the more stringent visibility
private, then protected and last public.
Think twice before adding a public member to a class.
Making something public make it harder to change afterwards.
Your
struct TransportTypeList {
const char *name;
TransportType type;
};
static const TransportTypeList transportTypeList[];
should really be protected if not private to
ObjectManagement.
current CERTI source code is not always (humm...)
a good example for the preceding practice but I think
that when new or refactored code is done we should try
to apply those rules.
Do not take those remark as "pure" criticism but
as an effort to provider better code base.
|