Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up code checking class invariants #28

Closed
morinim opened this issue Jul 4, 2020 · 1 comment
Closed

Clean up code checking class invariants #28

morinim opened this issue Jul 4, 2020 · 1 comment

Comments

@morinim
Copy link
Owner

morinim commented Jul 4, 2020

Classes should specify their invariants: what is true before and after executing any public method.

  • The function used to check the consistency of an object (debug()) should be renamed is_valid() (the most frequently used name);

  • Eliminate clutter and redundant checks.

    A common pattern to implement invariants in classes is for the constructor of the class to throw an exception if the invariant is not satisfied. Since methods preserve the invariants, they can assume the validity of the invariant and need not explicitly check for it.

    int A::method(B &b, int v)
    {
      Expects(b.debug());  // REDUNDANT CHECK
      Expects(v > 0);      // REQUIRED
    
      // ...
      // embarrassing code
      // ...
    
      Ensures(b.debug());  // REDUNDANT CHECK
      Ensures(v > 0);      // REQUIRED
    
      return v;
    }
  • The debug()/is_valid() method should exist only in the debug build, thus it does not add to code bloat.

References:

@morinim
Copy link
Owner Author

morinim commented Jul 4, 2020

The debug()/is_valid() method should exist only in the debug build, thus it does not add to code bloat.

It's handy to have the is_valid() method when compiling a unit test in release mode.

So the is_valid() method should be:

  1. called only in debug builds;
  2. available in release builds for unit testing.

1

is_valid() is used only together with macros Ensures, Expects, assert so it's not called in release builds.

2

Since only the first virtual function in a class increases its size (compiler-dependent, but on most - if not all - it's like this) and non-virtual functions don't affect the class size, it's not worth cluttering the source with code like:

#if !defined(NDEBUG) \
    || (defined(DOCTEST_LIBRARY_INCLUDED) && !defined DOCTEST_CONFIG_DISABLE)
#  define VITA_DEBUG_OR_TEST
#endif

class A
{
  // ...

#if defined(VITA_DEBUG_OR_TEST)
  bool is_valid() const;
#endif
};

#if defined(VITA_DEBUG_OR_TEST)
bool A::is_valid()
{
  // ...
}
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant