Coding_Standards

OWLNext Coding Standards

The OWLNext source code with sub-libraries (OCFNext, OWLExt, OWLFx and CoolPrj) consists of a large number of files written by various authors and has been revised over several years. Much of the code dates back to pre-standard C++ and simpler development tools. Inevitably the code is of varying standard. That said, we aim to improve on that going forward, and this page outlines our recommended coding standards for new and modified code.

• OWLNext Coding Standards
  • Naming conventions
  • Formatting and documentation
    • Use spaces, not tabs, for indentation
    • Do not align items vertically
    • Use a reasonable line length for readability
    • Follow the style of the surrounding code
    • Use a formatting style that is kind to outlining
    • Document code using Doxygen standards
    • Do not repeat documentation
  • Patches and version control
    • Follow the Subversion conventions
    • Keep unrelated changes apart
    • Provide helpful and complete log messages with tags
    • Commit to the trunk
    • Use the Subversion Merge Tracking feature
    • Merge often
    • Try not to break the build
    • All submissions must have an OWLNext compatible license
  • C++ best practices
    • Prefer signed to unsigned integers
    • Beware of truncation
    • Write standard C++
    • Resist conditional code
    • Write exception-safe code
    • Use assertions
  • References

---

Naming conventions

In general, OWLNext follows the naming convention in the original OWL source code. Here is a summary:

• Do not use leading underscores. Rationale: Names with leading underscores have some restrictions in the C++ standard. See C++ rules.
• Macros should be all upper-case, using underscore to separate words, e.g. "OWL_BREAK". New macros should have the "OWL_" prefix.
• Except for macros, all symbols should use mixed-case without underscores. Exception: A trailing underscore can be used for file-local names in implementation files, e.g. "TLocalSingleton_".
• All types should use the "T" prefix, with the type name capitalized, e.g. "TWindow". Exception classes should use the "TX" prefix, e.g. "TXWindow".
• Functions, structure (class/struct) members and global variables and constants should be capitalized; functions should start with a verb, e.g. "IsDirty", and variables and members should be nouns, e.g. "DirtyFlag". Exception: Function pointers and functor variables should be named as functions, i.e. with a leading verb.
• Function parameters and local variables begin with lower-case, e.g. "parent", "resId", "i".
• Template parameters are capitalized, e.g. "T", "N", "TAlloc".
• Enumeration members should have a lower-case type prefix, e.g. "wfAlias" (TWindowFlag). Rationale: Enumeration members are injected into the enclosing namespace, hence need to be differentiated in that namespace.
• Except for enumerations, do not use Hungarian notation (type prefixes). Rationale: Types may change. Also, these prefixes are hard on the eye and clutter the code. Modern editors give pop-up type information.
• Prefer short local names, e.g. use "w" instead of "window" or "wnd". Only use more descriptive names to clarify complex code, and then use names that describe role or function instead of type, e.g. use "parent" or "child" instead of "window". Rationale: Keep it simple. Less verbose code is easier to read.
• Abbreviations and acronyms within names should be capitalized as normal words, e.g. "EnableCtl3d", not "EnableCTL3D", and "OwlMain", not "OWLMain".
• File-local names, i.e. implementation details defined in the global scope in implementation files, should be put in an anonymous (unnamed) namespace. Such names should have a trailing underscore, e.g. "namespace {bool GlobalInitFlag_ = false;}". Prefer unnamed namespaces instead of the "static" keyword for file-local objects.

For more information about the original OWL coding style see the document "ObjectWindows Library Coding Style" ("doc/owlstyle.doc") in the Borland C++ installation. Note that some style items are outdated, such as the use of macros for exception handling, non-standard smart pointers, stack optimization and the use of separate inline functions.

---

Formatting and documentation

This section summarises our standards for code style and documentation.

Use spaces, not tabs, for indentation

New code should use 2 spaces per level of indentation. Rationale: A mix of indentation styles makes the code hard to read. Spaces provide consistent results, while tabs rely on the interpretation by tools.

Do not align items vertically

Although it is popular to line up code vertically, do not do this. Instead, use a sequential formatting style with line-breaks as you see fit. Rationale: Vertical alignment is brittle. See Vertical Alignment at Wikipedia.

The OWLNext source code is currently a good example of the mess that vertical alignment creates. This is one area where you should not follow the conventions in the existing code. We hope to gradually clean it up.

Use a reasonable line length for readability

Break lines at a reasonable length (e.g. around 130 characters). While it has been a tradition to keep source code line lengths within 80 characters (primarily for fixed terminal and printer limitations), this is unnecessary narrow in modern environments. But feel free to use more frequent line breaks for readability as you see fit, e.g.

void foo(
  TMyVeryLongType a, 
  TAnotherVeryLongTypeWithAnArgument<TSomeType> b,
  TMediumLong c, 
  int d);

Follow the style of the surrounding code

When you make patches and minor edits to existing source code, use the same formatting style as that of the surrounding code. If the existing style is indeterminate, or at odds with our guidelines here, follow the guidelines. Rationale: A mix of styles makes code hard to read. Tip: Put files with bad style on a "to-do" list for separate coordinated clean-up.

Use a formatting style that is kind to outlining

Your code and comment blocks should work well with outlining features in modern code editors. For example, the following documentation block style works well:

//
/// Does some foo related work.
/// More details about the foo function are imminent.
//
void foo();

The empty comment line before the Doxygen comment block creates a non-collapsible line between the comment block and the preceding code. It acts to clearly separate the comment block from a preceding collapsed definition. The empty comment line after the comment block provides spacing and makes it clearer that the comment block relates to the following code.

Rationale: Outlining makes it easier to work with source code.

Document code using Doxygen standards

OWLNext uses Doxygen to produce documentation for the libraries. You should follow our Doxygen Documentation Guidelines in your code. Since code comments go into the library documentation you should use generally accepted writing conventions, such as capitalisation and punctuation. This is the recommended style for all comments in general.

Do not repeat documentation

For functions and static symbols, document the declaration in the header file and not the separate definition and usage sites. For classes, document the definition and not the forward declarations and usage sites. For include files and macros, document the definition and not the usage sites.

Rationale: Repeated documentation is redundant, of little use and is likely to become out of sync.

---

Patches and version control

This section summarises our guidelines for submitted patches and checked-in revisions to the code.

Follow the Subversion conventions

The code repository has the conventional structure with top-level directories "trunk", "branches" and "tags". For more information about the practical benefits and use of this repository structure, read the Subversion documentation and tutorials.

Keep unrelated changes apart

Keep your patches and revisions focused; a single revision should be related to a single issue. Rationale: It is important for version control to keep issues separate, i.e. for branch merging and undoing revisions. It also makes it easier to follow the revision history and understand the changes made in each revision.

Often editing some code to fix an issue or add a feature reveals issues nearby. But don't succumb to the temptation to fix these issues in the same revision. Submit a separate revision instead.

Note that this recommendation even applies to seemingly small changes of the formatting or style of surrounding code. For changes related to formatting and style it is recommended to store these up for larger coordinated changes. Tip: Keep a record of such proposed changes and coordinate with the other developers for a major overhaul of the code.

Tip for TortoiseSVN users: Always thoroughly inspect your changes in TortoiseMerge before you commit, e.g. using the command "Compare with base" in the Commit dialog, or more safely, using the commands "SVN Diff" and "SVN Check for modifications" from Windows Explorer. If you spot unintended changes, you can undo them directly in the viewer (commands "Use other text block" etc. are helpful for this purpose). If you spot unrelated changes that you want to commit separately, then you can temporarily undo them in the Commit dialog by using the "Restore after commit" command. Then undo these changes in TortoiseMerge before you commit. After the commit, TortoiseSVN will restore the file so that you can commit the unrelated changes separately, if appropriate.

Provide helpful and complete log messages with tags

Never submit empty log messages to the code repository. Provide a helpful and complete log message detailing your changes. Prefer full sentences, and always end the message with a period/full stop. This makes it clear that the message has not been truncated.

Use one of the following tags to prefix the description of each change; for example:

String enhancements for the TFoo class:

NEW: Added an owl_string overload to TFoo::GetString [feature-requests:#123].
CHG: Elaborated on the documentation of TFoo::GetString.
BUG: TFoo::GetString causes buffer overrun [bugs:#456]. Resolution: Added buffer check.
BUG: TFoo::Send fails for Unicode (regression in [r789]). Resolution: Wrapped strings in the _T macro.
RGR: Build fails after use of std::optional in "dc.h" [r5016]. Resolution: Included "gdiobjec.h" (complete types).

TODO: Investigate the purpose of TFoo::Contrived; seems not needed anymore.

If the submission consists of several changes, as above, then add a header that describes the overall purpose or context for the changes. If you cannot provide a good header, it is probably an indication that the submission bundles unrelated changes, and you should split it up into smaller orthogonal submissions.

• NEW: When describing new features, refer to the ticket in the request tracker, if any, using the format "[feature-requests:#123]". Rationale: This allows version control systems to link the revision with the issue tracker automatically.
• CHG: This tag will apply to most changes that are not new features nor bug fixes. Since this tag rarely refers to the tracker, it is especially important that the log message is helpful and complete. Don't save space!
• BUG: Note that this tag is short for "I implemented a bug-fix for the following issue", so "BUG: Fixed the buffer overflow in Foo" is redundant. Simply use "BUG: Buffer overflow in Foo" instead. Describe the bug as it was, i.e. copy the ticket title from the bug tracker, and append a ticket reference using the format "[bugs:#123]". If the bug is not in the tracker, but it is known that the bug was introduced by a change in a specific revision, i.e. the issue is a regression, then add a reference to the guilty revision as shown in the example above. If neither applies, then be as specific as possible about the nature of the issue. Then, if it is not obvious, add a description of the resolution as shown in the example above.
• RGR: This tag applies to regressions; issues that appear due to code changes, e.g. the build failing. Use this tag rather than BUG when you want to point out that the code used to work just fine and that a particular change broke it. In the log statement, refer to the change that caused the regression, with a reference to the revision number, and then state the resolution of the issue.

Feel free to add comments and todo-items at the end of the log message. This may help other developers review and further improve the code.

Exception: Changes entirely related to version control can omit the tags.

Tip: Log messages can be edited, so review, correct and improve log messages as you see fit.

Commit to the trunk

Cutting edge development happens on the trunk of the repository, and normally revisions are committed there. Revisions to release branches need more care and should usually be handled by, or cleared by, the release manager assigned by the project administrators. Consider committing the revision to the trunk or submitting a patch instead.

Rationale: Release branches need to be carefully handled to provide stable releases to end users.

If you cannot work on the trunk, e.g. you work on a patch for a specific release, then create a development branch as a copy of the specific release branch, and then commit your work on the development branch. When work has completed, the release manager will eventually merge the development branch back into the release branch (for a patch) or create a new release branch (for a major/minor version bump) in preparation for release.

Use the Subversion Merge Tracking feature

Subversion provides merge tracking functionality that records which revisions have been merged from one branch to another. This information is recorded in the svn:mergeinfo property. You should normally always use the merge functions to keep this information correct and up-to-date. If you manually merge changes, or want to exclude a revision from being merged, you need to manually update svn:mergeinfo accordingly. Rationale: The merge feature automates and simplifies merging.

When committing a merge to the repository, provide a log message starting with "Merged [rREVNUM] from path to path:", then followed by a copy of the log messages from the revisions merged. For example:

Merged [r1001] from trunk to branches/6.44:

NEW: Added blah blah blah.
CHG: Changed blah blah blah.
BUG: Blah blah blah is nonsense.

Feel free to abbreviate long messages. The point is to get an overview of the changes merged, including links to related revisions and tickets. E.g. if the source revision has a lot of changes, you can provide a summary instead.

Rationale: A complete log message makes browsing the repository log easier, and makes it possible to recreate the merge history, should the svn:mergeinfo property be corrupted.

Merge often

When there is work proceeding on several branches, you should merge often. Rationale: Frequent merging can prevent large complicated merges with lots of conflicts.

Exception: If work is proceeding on mainly one branch, or affect different parts of the code, thus posing little threat of conflicts, then it can be beneficial to accumulate revisions before merging. Still, include only related changes in each merge. Rationale: A single merge consisting of a batch of revisions makes a simpler log.

Try not to break the build

Code checked into the repository should compile, ideally on all supported compilers. We recommend using OWLMaker to test the build. Rationale: Breaking the build hampers other developers.

Of course, in addition to merely ensure compilation, you should test and verify that the submitted code works as intended.

All submissions must have an OWLNext compatible license

Submitted code must carry all the relevant copyright statements as well as a license statement compatible with the OWLNext license. Rationale: Unlicensed code and code with an incompatible license can not be accepted for legal reasons.

---

C++ best practices

This section summarises some of the items of particular relevance to OWLNext. For general rules, guidelines and best practices for writing C++, we recommend the "C++ Coding Standards" by Sutter and Alexandrescu [1].

Prefer signed to unsigned integers

In general, only use unsigned integers when you need modulo arithmetic or bit manipulation. Otherwise use signed integers. Rationale: Integer conversions and promotions can cause non-intuitive behaviour. See the Google Style Guide for more in-depth discussion.

Exception: An unsigned integer is acceptable for representing identity (e.g. message ID) when no arithmetic and no comparisons, other than equality, make sense on the type.

Beware of truncation

In general, explicit casts may introduce truncation with resulting unintended or undefined behaviour. Casts should hence be avoided, if possible. However, they are often required. Since the standard library uses size_t or equivalent types, and other APIs may use uint or similar, that all have a range outside int, there is often the need to cast a size to int (with the previous guideline about using signed sizes in mind). But you should always check that a type conversion makes sense and does not truncate. Rationale: Truncation is rarely insignificant.

To assist in checking for trunctation, OWLNext provides a helper function to check that a value lies within the range representable by int (or any other specific integer target type). This predicate is called IsRepresentable (defined in "owl/defs.h") and takes the target integer type as a template parameter. If the function returns true, you know that a cast to the target type will not truncate.

Usage example:

inline auto TDC::GetTextExtentPoint32(const tstring_view& str, TSize& extent) const -> bool
{
  PRECONDITION(str.data() && IsRepresentable<int>(str.size())); 
  PRECONDITIONX(std::find(str.begin(), str.end(), _T('\0')) == str.end(), 
    _T("Null-terminator detected within string (str.size() > _tcslen(str.data()))."));
  return ::GetTextExtentPoint32(GetAttributeHDC(), str.data(), static_cast<int>(str.size()), &extent);
}

Likewise, beware of trunction of strings when dealing with string buffers of fixed size. To avoid string truncation issues, prefer to use dynamic string encapsulations, such as owl::tstring and owl::tstringstream (which may allow you to simplify the code and make it more robust at the same time). Rationale: Again, truncation is rarely insignificant.

Write standard C++

Do not use non-standard compiler extensions or library features. Rationale: We aim for standard C++ compliant code to simplify maintenance and increase code compatibility. Exception: Restrict your use of standard C++ to what is supported by all the compilers on our Supported Compilers list.

Resist conditional code

Try to use common code for all supported compilers and build modes. Rationale: Conditional code is hard to read and maintain.

Exception: Sometimes conditional code is unavoidable. In these cases you should factor out the conditional code by using abstractions (such as type definitions and macros). E.g. define a type definition conditionally and use it where needed, rather than inserting conditional code at every usage site.

For example, don't do this:

#if defined(MessageBox)
#undef MessageBox
inline int MessageBox(HWND hWnd, LPCTSTR lpText, LPCTSTR lpCaption, UINT uType)
#if defined(UNICODE)
{ return ::MessageBoxExW(hWnd, lpText, lpCaption, uType, 0); }
#else
{ return ::MessageBoxExA(hWnd, lpText, lpCaption, uType, 0); }
#endif
#endif

#if defined(GetFileTitle)
#undef GetFileTitle
inline short GetFileTitle(LPCTSTR lpszFile, LPTSTR lpszTitle, WORD cbBuf)
#if defined(UNICODE)
{ return ::GetFileTitleW(lpszFile, lpszTitle, cbBuf); }
#else
{ return ::GetFileTitleA(lpszFile, lpszTitle, cbBuf); }
#endif
#endif

Do this instead:

#if defined(UNICODE)
#define OWL_TNAME(n) n##W
#else
#define OWL_TNAME(n) n##A
#endif

#undef MessageBox
inline int MessageBox(HWND hWnd, LPCTSTR lpText, LPCTSTR lpCaption, UINT uType)
{ return OWL_TNAME(::MessageBoxEx)(hWnd, lpText, lpCaption, uType, 0); }

#undef GetFileTitle
inline short GetFileTitle(LPCTSTR lpszFile, LPTSTR lpszTitle, WORD cbBuf)
{ return OWL_TNAME(::GetFileTitle)(lpszFile, lpszTitle, cbBuf); }

Write exception-safe code

Unlike Windows API C code, OWLNext is C++ code, and you should expect that any function and constructor may throw an exception, unless it is explicitly stated in the API that it does not (i.e. the function signature has noexcept applied, or the now deprecated throw() specification).

Since an exception may occur on almost any program statement, resources may leak, unless you carefully write your code so that it always cleans up, either by explicit exception handlers, or much more smartly, by using C++ code constructs, such as smart pointers, that exploit RAII to deallocate resources automatically.

Read more in our wiki article Exceptions and OWLNext.

All new functions added to OWLNext should prefer exceptions to error return values for error handling.

Regarding coding style, prefer the "early exit" style in favour of nested control-flow, i.e. a "flat" coding style with minimal indentation, focussing on the main logic of the function rather than the error handling.

For example, consider this function:

auto TOWLMakerEdit::FileSaveAs() -> bool
{
  if (TCoolEditFile::FileSaveAs())
  {
    const auto a = dynamic_cast<TOWLMakerApp*>(GetApplication()); CHECK(a);
    a->SaveMenuChoice(GetFileName());
    StartWatchingFile();
    return true;
  }
  else
    return false;
}

Unfortunately, rather than using exceptions for error handling, the base function TCoolEditFile::FileSaveAs is defined with an error return value. This forces us to include control flow to handle the error value. So, although this function is small and easy to understand, the control flow and indentation of the function is wholly driven by the error handling, obscuring the main purpose of the code, which is simply to add some functionality to the base, i.e. call SaveMenuChoice and StartWatchingFile.

In this case, it is better to use a flat coding style that approximate exception handling, using an early return statement if the base call fails:

auto TOWLMakerEdit::FileSaveAs() -> bool
{
  const auto r = TCoolEditFile::FileSaveAs();
  if (!r) return r;
  const auto a = dynamic_cast<TOWLMakerApp*>(GetApplication()); CHECK(a);
  a->SaveMenuChoice(GetFileName());
  StartWatchingFile();
  return true;
}

Now, if the base should later be refactored to use exceptions instead of error return values, then TOWLMakerEdit::FileSaveAs would require just minimal changes:

void TOWLMakerEdit::FileSaveAs()
{
  TCoolEditFile::FileSaveAs();
  const auto a = dynamic_cast<TOWLMakerApp*>(GetApplication()); CHECK(a);
  a->SaveMenuChoice(GetFileName());
  StartWatchingFile();
}

Note that the changes do not touch the main logic of the code, and require no changes to the block structure (indentation and braces).

Use assertions

Use copious amounts of assertions in the code. Check preconditions, local assumptions and post-conditions.

Rationale: Assertions detect misuse by the user, e.g. the passing of illegal arguments. They also detect misconceptions by the programmer, while at the same time documenting programmer assumptions and presumptions. The latter is an important part of robust and maintainable code.

Assertions on parameters should use PRECONDITION at the top of the function. The same goes for asserting the state (invariant) of the object (this). CHECK is only used within the function to check (and thus implicitly document) local assumptions, e.g. about what other functions called return, as well as post-conditions, what you expect after all is said and done. For example:

bool TNoteTab::IsFullyVisible(int index) const
{
  PRECONDITION(GetHandle());
  PRECONDITION(index < GetCount() && index >= 0);
  if (EffectiveTabsArea.IsEmpty()) return false;
  if (!IsVisible(index)) return false;
  TRect b = GetBoundingRect(TabList[index].Rect);
  CHECK(!b.IsEmpty());
  return ContainsHorizontalExtents_(EffectiveTabsArea, b);
}

For more information on the diagnostic macros within OWLNext, see Diagnostic Macros in the OWLNext Documentation and the wiki article Built-in Diagnostic Window in OWLNext. Also see the Knowledge Base.

---

References

1. "C++ Coding Standards", Herb Sutter and Andrei Alexandrescu, Addison Wesley, 2005.