My suggested clarifications/changes:
- for “private class”
- inherit as private
- m_parent if necessary; but probably shouldn’t be because:
- there should be non public members in the public class?
- Functions in the public class are either static or shims to the private class’s functions
- all methods and members should be private to make “private class” intent and encapsulation clear
- For corresponding public class:
- The private and protected sections of the public class should be minimal, ideally only containing private: m_private
- exposing this detail in the header for the public class is unnecessary and leads to unnecessary recompilation due to non-functional changes to the header file
- see also https://en.wikipedia.org/wiki/Facade_pattern
- The private and protected sections of the public class should be minimal, ideally only containing private: m_private
- request for comment: m_child & m_parent or m_private & m_public (my preference)?
- Doxygen comments, multiline do not prefix lines with “ *”
- I don’t really have that strong of a preference here, but I still think the “ *” is unnecessary.
- /** on its own line (how most are now) so the comment text is all aligned and starting at the column of /
- filenames
- request for comment:
- the files seem to be all lowercase (compatibility with Windows?), so allow use of - or _ to aid readability? Or use mixed case?
- for programs, use program.cpp instead of main.cpp, e.g. mythfrontend.cpp
- this makes it obvious which file or program is referred to
- e.g. when gcc fails to compile, it prints main.cpp, which must be disambiguated
- also the generated doxygen graphs
- this makes it obvious which file or program is referred to
- (libmyth*) libraries
- name export header as libraryexport.h, e.g. libmythexport.h instead of mythexp.h
- the current abbreviations are unnecessary and reduce clarity
- is there an interface definition for these (internal) libraries somewhere?
- avoid prefixing the filenames with myth; it is unnecessary since the file is in libmyth*
- some are probably fine, but most seem gratuitous
- request for comment: when including the header files, include the lib*/ part of the path
- being explicit here eliminates having to search for the files and also makes it easy to sort the includes, e.g.:
- C/C++
- POSIX or other system libraries
- Qt
- libmyth* (each lib separately and in order of dependency tree)
- being explicit here eliminates having to search for the files and also makes it easy to sort the includes, e.g.:
- name export header as libraryexport.h, e.g. libmythexport.h instead of mythexp.h
- request for comment:
Thanks,
Scott