Anti-Patterns | Developer Documentation

Anti-Patterns

Introduction

The purpose of this document is to provide the guidelines for how not to write code for the KiCad project. Anti-patterns are code practices that seem to occur often but shouldn’t. If your code contains any of these patterns, please fix them.

Do not modify this document without the consent of the project leader. All changes to this document require approval.

Dereferencing Pointers

Dereferencing an invalid pointer almost always results in a crash. Never assume a pointer is valid. Even if the code in question has never crashed before, that doesn’t mean that someone wont break something in the future. Use assertions to help developers catch invalid pointers during development. Even better than assertions, you can use the wxWidgets debugging macros which can raise assertions on debug builds and gracefully handle run time issues in release builds. If you are concerned about pulling in wxWidgets user interface code, there are header only C++ assertion libraries available.

Avoid the temptation to silently hide potentially bad pointers. The following code:

void SomeFunction( SomeObject* somePointer )
{
    if( somePointer )
    {
        somePointer->DeReferencedMethod();
    }
}

should be replaced with:

void SomeFunction( SomeObject* somePointer )
{
    assert( somePointer )

    somePointer->DeReferencedMethod();
}

wxUpdateUIEvent Abuse

Avoid using wxUpdateUIEvent for anything other than the methods provided by the event object. Updating controls in a wxUpdateUIEvent handler will typically trigger other control events that in turn will fire more wxUpdateUIEvents which results in an infinite event loop that can cause the user interface to become less responsive.

If you are not sure you are abusing wxUpdateUIEvent, the advanced configuration setting UpdateUIEventInterval allows you to set the time in milliseconds which the update events are generated or -1 to disable all update events. If slowing or disabling the update events makes the user interface more responsive, a wxUpdateUIEvent handler is the problem.

Translating Partial Sentences

Never break an translated sentence into multiple _() tags. This prevents translations from understanding your intention and from creating grammatically correct sentences in languages other than English.

For example the following is incorrect:

ShowReport( _( "Duplicate instances of " ) + m_changeArray[j].NewRefDes );

This should be replaced by the following:

ShowReport( wxString::Format( _( "Duplicate instances of %s" ), m_changeArray[j].NewRefDes ) );

The new formulation allows a translator to move the object of the sentence. In the original formulation, the object (the NewRefDes string) would always appear at the end of the sentence, which may not be grammatically correct in non-English languages.

Similarly, do not break sentences into parts like this:

wxString verb  = updateMode ? _( "Update" ) : _( "Change" );
wxString warning = _( "%s all items on the board?" );

wxMsgBox( wxString::Format( warning, verb ) );

In English, the word Update is both a noun and a verb. Without context, the translator will not know which version to use. Additionally, it will not be clear to the translator that the verb string is used by the warning string as they will not appear together in the translation file.

Instead, we must write the sentences explicitly:

wxString msg = updateMode ? _( "Update all items on the board?" ) :
                            _( "Change all items on the board?" );

wxMsgBox( msg );