Tech/LxEngine/Coding Style

From Athile

Principles

• Clarity of code should be foremost; only sacrifice for efficiency in proven bottlenecks
• Prefer concise and clear over general and flexible
• Prefer free-standing functions to class methods
• Use "::detail" sub-namespace to hide implementation details that client code should not need to be aware of
• Avoid implicit operations: implicit casts, implicit single-parameter constructors, etc.
• Reduce dependencies
• Consistency
• Reduce the number of concepts
• Use standard, familiar concepts, patterns, paradigms, and names
• Reduce special cases and details
• Comment extensively; comment on the higher-level intent of the code not a literal transcription of what the code does

If it's complicated, it's not done yet - That's a good principle for the code. It's alright for a first revision of some code to be a bit complex because it takes work to make things simple. The key idea to remember is that the code is not done when it 'works'; it needs to be iterated over until it is simple.

Conventions

• ClassPtr = std::shared_ptr<Class>
• ClassCPtr = std::shared_ptr<const Class>
• ClassWPtr = std::weak_ptr<Class>
• ClassCWPtr = std::weak_ptr<const Class>
• kVarName = constant value
• eVarName = enumeration value
• g_varName = global scope variable
• s_varName = static scope variable
• mVarName = member variable
• spVarName = smart-pointer to variable
• wpVarName = weak-pointer to variable
• pVarName = raw pointer to variable
• _methodName() = intended for internal use only, usually protected methods
• 'internal:: = sub-namespace for internal use only classes
• detail:: = sub-namespace for implementation details that rarely relevant to the client
• #include <../detail/file.hpp> = header for internal use only

• Line length: lines, from their start (which likely is not column 0), should be 80 characters (preferable) to ~120 (when necessary)

Classes and Concepts

LxEngine developers should be familiar with the following classes and concepts and, for consistency, use them when appropriate (rather than alternate implementations):

Detail

File Layout

module\
    group\
        submodule\
            functions.hpp
            functions.inl
            class0.hpp
            class0.inl
            class1.hpp
            class1.inl
            ...
        submodule.hpp

Robust, Non-Performance Critical Function

//---------------------------------------------------------------------------//
// function name
//---------------------------------------------------------------------------//
//! brief description
/*!
    \ingroup module_group_submodule
 
    Here is the long description of the function.  More details. Etc.
    Etc.
 
    Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
    tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
    veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea 
    commodo consequat. Duis aute irure dolor in reprehenderit in voluptate 
    velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint 
    occaecat cupidatat non proident, sunt in culpa qui officia deserunt 
    mollit anim id est laborum.
 */
static inline
float
my_function_name (Object* pObject, int a, float b, float b)
{
    //
    // Check pre-conditions before potentially modifying any persistent state.
    // Want to potentially throw exceptions only with transient state modifications.
    //
    lx_assert (a != 7, "Do not pass in 14, not 7.  The result will be the same but faster");
    lx_check_error(pObject != nullptr);
    lx_check_error(pObject->method1() != false, "Object in incompatiable state");
 
    try
    {       
        int intermediate = pObject->doSomeWork(a, b);
        for (int i = 0; i < a; ++i)
        {
            intermediate *= pObject->doMoreWork(a);
        }
        pObject->finishWork(intermediate * c);
 
        lx_check_warn(intermediate == 0, "Object intermediate was zero");
        lx_assert(pObject->statusComplete() == true);       
    }
    catch (lx0::error_exception& e)
    {
        e.location(__FILE__, __LINE__);
        e.detail("Failed to compute widget factor delta");
        e.detail("a=%1%, b=%2%, c=%3%", a, b, c);
        throw;
    }
    catch (l0x::fatal_exception& f)
    {
        _write_minimal_emergency_crash_data(pObject);
        f.detail("Object data saved to crash file");
        throw;
    }
}

Todos

Ideally todos should be in the following format:

void 
MyClass::myMethod (int arg1, int arg2)
{
    ///@todo Implement myMethod to <short summary of implementation>
    /*
        This should be a longer description of what the implementation
        involves.  It should include as much description as possible
        while the issue is fresh in mind.
 
        Multiple paragraphs are fully warranted in this case since
        if it was minor, it should not be left as a todo.  If it is not
        minor, then it likely requires some description - namely the
        challenging aspects that caused the item to be left as a todo.
    */
    lx_error("Code not implemented!");
}

There are four parts to the above:

• The stub method / class / section of code that defines exactly where the to-be-implemented code should go. Don't necessarily enforce a design on the implementation, but do provide the location where that implementation will be written or called from. The more descriptive a name that can be assigned to the stub method, the better.
• The Doxygen todo comment provides a short summary for developers looking for tasks
• The full description provides a valuable description. It is essential to write this rather than simply leaving a vague todo. At the time the todo is being written, the developer has a great deal of useful information for a future developer (even himself) in the context of the work being done. This section can be written as quickly and roughly as necessary, but should provide as much information as possible on what specifically needs to be done.
• The error: this provides the runtime notification that an incomplete part of the system has been invoked. In some cases, a warning may be acceptable if the todo is truly supplemental and not essential functionality

Glossary

• ensure - uses to prefix a method or function on a cached resource or state change that, if the resource is ready or the state already set will do minimal work, otherwise it will set that state or ready that resource

Issues

Use size_t or int for values that shouldn't ever negative?

Example: should Image::width() return a size_t or an int?

Response: