OpendTect software development rules

Software engineering is a game of trade-offs. Performance vs generality, flexibility vs stability, priorities vs general goals, and so on, and so on. A good software engineer weighs all pros and cons and comes up with (near-)optimal solutions, often trying to get the best of everything. Of course, in fact, sometimes things can be completely ignored (e.g. in dialog-UI's performance is seldomly an issue).

Goals

When creating and maintaining software code, invariably one wonders what choice will come out optimally. To define 'optimal', we have to define the goals we want to maximise. The most obvious is:
(1) Total time spent (man-hours)
Less obvious, but also very important is:
(2) Total programming pleasure
The issue is that we software developers have to 'live' in the code, for many hours each day. Nothing is more dissatisfying than to have to go through code that looks like crap, even though the code may be working. Not being able to find what you need, is another issue. Too complex code, too. Finally, maybe an item not really fitting in the list, but still:
(3) Team-readyness
If you find pleasure in typing 15-level '?'-statements - so be it. If your entire team likes it there is no problem either. But chances are you will be crucified by your team members when they have to do anything with your code.

Nice and neat

Good code should look good. You have to find joy in making the things you deliver look as good as (reasonably) possible, and as easy to understand as possible. Compare these two class definitions:

class SizeKeeper
{
public:
		SizeKeeper() : sz_(0)		{}

    int		size() const			{ return sz_; }
    void	setSize( int n )		{ sz_ = n; }

protected:

    int		sz_;
};

and:

class X { public: X() : n(0) {}
protected: int n;
public: int N() const { return n; } void sn(int p) { n = p; } };

The second class definition looks like crap, and is difficult to understand, especially when you imagine the code where the class is used. Maybe the first definition isn't that clear for everybody immediately at the start, but it's easy to see that once you get used to the style, it will be easy and fun to work with this kind of code.

For this to work, a team must agree on a style. The style characteristics are chosen so they match the requirements of esthetics/pleasure and time minimisation.

It may look like the second class definition has an advantage over the first in the time spent creating it. Nothing is more true. Time in software development is spent on many things, and actually typing the code is just a tiny component:

• Time spent typing
• Time spent thinking/designing/creating
• Time spent changing
• Time spent understanding
• Time spent reworking
• Time spent debugging

For most practical purposes, the influence of typing time on the total time spent can be neglected.

That leads to an important principle:
Rule (1): Make your code look good right from the start.
Waiting for a clean-up stage is a serious mistake. Already during creation of the software, from the very start, the effects of sloppy code will hit you where it hurts. Even if you have to re-type sections 10 times, it is better to have the code really neat at all times. Only then you can see that the re-working is necessary. The earlier you detect that constructions are not intuitive, logical, and easy to understand the earlier you detect that your code is actually bad.

Uniform

This is not a point to be taken lightly. Other team members will at some point have to change your code, other team members will at some point have to debug your code. Uniformity makes sure this is as easy as possible. Remember this: changing code you haven't made yourself is never easy, so do everything you can to help your team members. Moreover, changing code you've made some time ago is never easy, so you're even doing it for yourself.

The implications are simple although a lot of programmers have a lot of problems with it:
Rule (2): Make your code look just like all the other code.
Combining (1) and (2) could casually be described as: make sure all team members feel at home in your code at all times.

Simple & Easy

Any complex process can be broken up into simple steps, any complex object can be broken up into simple objects. Always consider yourself as publishing something that needs to be read by others. Take them by the hand and make it easy to understand what you are doing, and why you are doing it. Avoid repetitions, complex constructions or long lines. Make things compact if that will make things clearer, or uncompact if needed.

The best code you will ever make will invariably look as if it has cost hardly any time to make. Like good dancers make it look like there is no effort involved, an excellent solution always looks simple, compact and easy to use.

The way we do things in OpendTect is not a 100% fixed body of rules. Moreover, we tend to say 'rather do this than that', or sometimes we change our point of view. Still, we almost unanimously agree on almost every issue. To lower the time to discover how we do things, next to going through lots of code, you can make use of the rules that follow.

OO and general rules

• Try to avoid pure implementation inheritance. Inheritance of 'mainly interface' is usually OK. In all cases, ask yourself whether there really is a 'isA' relation between the classes. Prefer delegation in any doubt.
• Be very aware of dependency management. Avoid using services from classes that were designed for something else. In doubt, split that class into the common part and the part that you are not interested in.
• Anything adding to the complexity has to be justified. Don't use patterns or other nifty tricks without a good reason. Certainly, there are often good reasons. Factories for example are almost always there for a good reason. But, always ask yourself: is it worth the extra effort? The simple alternative may not be as flexible, but do I really need that extra flexibility?
• Do not implement anything that isn't used (yet). Don't go for 'complete classes' or that kind of mumbo-jumbo. Figure out which methods are indispensible (like copy constructors) and then add functions when they are needed. Things that seem to be sure to be used tend to never be used, instead they add to the burden of the maintainer. Sometimes pre-cooked stuff is removed in a re-work without ever being used. On the other hand, if you think something is needed later, you need to design the interface in such a way that if necessary, it is not unnecessarily hard to add. 'Prepare, don't implement'.
• Jokes and surprises are not funny. They may seem to you at the time but they are not. A mildly ironic comment once a year should be enough.

C++ rules

• We do not use exceptions. Exceptions are the horror story of C++, try looking at C++ report, November-December 1994, Tom Cargill: "Exception handling: A false sense of security". There are more reasons, for example that you have to use a certain paradigm throughout: RAII (Not a bad principle but not always easy to do and never enforcable). If you want to I can explain a lot of those reasons by e-mail. The bottom line is: don't use exceptions. External software using exceptions must be isolated with try { } catch (...) {}.
• We are not using all the STL stuff and the std::string class. This is not because we don't like it, but more because we don't see the need. Using this is not a problem but will not work well with the rest of the system so in general the classes are not used. For external software try moving to our own classes as quickly as possible and beware of problems with exceptions.
• All code must be const-correct except in specific areas where that would not give any gain: there it's optional. Learn the subtleties of const in various places. Don't cast away const unless you are certain about it, consider the possibility you need to use 'mutable'. Caching variables etc. should always be declared mutable. In OD, GUI classes and classes working with legacy stuff can be non-const correct. We do make them const-aware, which means they smoothly work together with classes that are const-correct.
• Operator overloading can be used very sparingly, in situations of simple classes with absolutely trivial usage. In any doubt, don't use it. It does more harm (sometimes a lot more) than it returns benefit. Even the ubiquitous examples like matrix calcutations are almost surely better made with good old-fashioned method calls.
• In cases that you don't know whether a language feature can be used, do not give the feature the benefit of the doubt. You can always ask your team members first. A C++ language feature should only be used if you can prove that it is useful, clear, fitting in our style and not easily possible with other means.
• We increasingly try to use name spaces. In many places namespaces should have been used and this is a legacy problem which we want to gradually get rid of.
• Do not pollute with things that are not C++, like M$-windows directives. If absolutely unavoidable design a strategy to minimise the impact of these horrible things.
• Consider implementing in a header file only if unavoidable (templates), or:
• Is the implementation stable? If not, dependencies will trigger each time the implementation is changed.
• The implementation must be completely trivial or useful for a reader. In the latter case, it replaces comments with something that is fundamentally up-to-date.
• The space taken may not be huge - then implement in the .cc file anyway.

Semantical/typographical rules

First of all: the naming of classes, variables, namespaces, etc. is extremely important. You want to optimise understandability and compactness, in doubt always go for understandability. Naming should be as intuitive as possible. If you cannot find an intuitive name, consider the possibility that your design is not right. Well designed classes and methods hardly ever have non-intuitive names. If you are really convinced you're right but still you can't find anything intuitive, make sure you explain the meaning in comments.

• Classes and name spaces have a well-chosen name. Very well chosen. Do not rest before you have a name that really suits the class well. Name spaces tend to have short names, classes tend to be longer. If you cannot find a good name you probably have to split up or redefine the class. Typographically, every syllable of the class/namespace name starts with an upper case character.
• Class methods also need carefully designed names. First of all, we have adopted the early Smalltalk rules: * First syllable: all lower case * further syllables: start with upper case. Then, how the method is named is dependent on what it does. The rule is that the resulting code must read as if it is English text, and that it does what it says. most often verbNoun is OK. Bad are:
  • bool moderator() - bool functions must be usable directly in 'if' or '?' statements. Imagine 'if ( moderator() )'. Depending on what it does, consider 'isModerator()' or 'moderate()'.
  • void wordChanger() - a word changer could be an object but not a function. Consider 'changeWord()' or 'changeWords()' or a re-design.
  • int applesAndPears() - what does this thing do? Make sure there is at least one verb.
• Variables are in lowercase. Class members should get an underscore at the end, further variables should generally be free of underscores. Special cases are Keys, 'hard' constants and Notifier names. There is a namespace 'sKey' and there are variables 'sKeyXxx' for key strings. Examples are 'sKey::Yes' and 'sKeyTraceLength'. Hard constants are like 'cMaxNrPatches'. Notifiers are defined in Basic/callback.h.
• Macros are like constants but with prefix 'm': 'mErrRet(msg)'. As usual, inline funtions, constants and templates are preferred but macros are still indispensible in real C++.

Layout

No other subject brings up this many discussions. While it's simple: choose a policy and stick to it. The end result is what counts: readability, compactness, understandability. Thus all rules can be broken if it really helps those properties, but they rarely are.

• Indentation:
  4 spaces per level, 8 spaces = one tab. Use tabs whenever possible, also inside a line.
• Alignment:
  The maximum number of characters on one line is 80. So when you exceed this number, start on the new line with a couple of tabs. Align function arguments as much as possible.

  		MyClass::functionWithLongName( const char* arg1,
  					       const char* arg2 ) const
  	

  When implementing functions in header files, align the implementations.

  		getPtr() const		{ return ptr; }
  		getValue() const	{ return val; }
  	

• Braces '{ }':
  On a line by themselves.

  		if ( b )
  		{
  		    stmt1();
  		    stmt2();
  		}
  	

  Single statements need not be braced.

  		if ( b )
  		    stmt1();
  	

  Two or three small statements may be packed on one line with braces.

  		if ( b )
  		    { stmt(); return; }
  	

• Parentheses:
  Pad with a space on both sides, except when this would make things unclear because it would become too spacy.

  		if ( x )
  		if ( (x && y) || (z1 && z2) )
  	

• Array brackets:
  No space between array and first bracket. Pad index if that makes things more clear.

  		x = arr[0];
  	

• Equality-type operators:
  Pad with spaces, unless it really helps seeing the grouping.

  		if ( a == b )
  		x = 15;
  	

  If more clear, use:

  		if ( c<d && e>f )
  	

  rather than:

  		if ( c < d && e > f )
  	

• Semicolons:
  Attach to last character of statement:

  		doIt();
  		for ( int idx=0; idx<10; idx++ )
  	

• '?'-statements:
  Use only and always if the same thing must be done or used depending on a condition:

  		return isOK() ? 10 : 20;
  		x = a > 10 ? 10 : a;
  		prTxt( isOK() ? "Yes" : "No" );
  	

• Class declarations:
  Just look at examples, but a nice template may be:

  class Y;
  class Z;
  
  namespace X
  {
  
  class A : public B
  {
  public:
  			A( const C& c )
  			: B(c)
  			, var_(0)		{}
  
      enum Type		{ T1, T2 };
      void		setType(Type);
  
      bool		isNice() const		{ return true; }
      bool		isOK(float) const;
      			//!< will not handle values < 0 well!
  
      void		doIt(int base_count,const Y&);
  
  protected:
  
      float		var_;
  
  private:
  
      void		init();
      friend class	Z;
  
  };
  	

  Remarks:
  • The tab alignment can be 2, 3 or 4 tabs, dependent on the length of things.
  • functions implemented immediately get a normal space padding for the arguments. Functions only declared get no padding for the arguments. Put variable names only if it adds to the understanding.
  • Comments can help but can also make things a mess. Use them sparingly, only to clearly specify what a method does, or to indicate pre-conditions etc.

From now on, new code will be as described above. What to do if the code you're changing is not good according to these standards? That depends on the amount of work vs the amount of time vs the importance of the deviation.

Adapting code

The rules are:

• Make sure the code is, after you're done with it, 'internally consistent'. For example, when adding a member to a class with already 20 members without an underscore you will likely add a member without an underscore, rather than changing all the other members. In no case make your new member the only one with an underscore. That is even more confusing to the reader of the .cc code.
• If you doubt whether to re-do parts, give changing it the benefit of the doubt unless it's really horribly dangerous to do so. But the rule is: In doubt => Change.
• Functions or members that seem to be unused but non-trivial: always try removing them or flagging them with messages (e.g. copy constructors and the like will be made by the compiler after you remove them and then you still don't know whether they are used). An unused non-trivial fuction must be considered 'unbearable' (see next item).
• Some things are 'unbearable'. Unbearably bad naming. Spaces instead of tabs. Unused code. Replace this kind of stuff where you see it. If these things happen too much, consider asking the producer of the crap to remove it him/herself.