As usual I could not find a relevant image on the magical world of google images, so I just jammed this cat meme in, because why not. Anyway, in this post I want to go over about something I have been thinking about recently, and that is my personal coding guidelines. I have had a number of influences on deciding these including learning from my peers, working on a variety of personal and professional projects, reading a variety of blog posts and most recently reading Framework Design Guidelines, a book which I think all .NET developers should read! A key consideration included "what if a junior programmer came onto the project, could they easily understand it?".  This is not final, these are just my personal guidelines up to this point and will likely change as I continue my programming career.

Naming of Variables, Properties, Functions, Classes and Structs

The following is the style I like to use:

Variable/Field names you can actually understand

I have come across a couple of code files where I have seen variable names in a variety of places named something like "int sss". Essentially when I start seeing this, this happens:

Variable and Field names should be verbose, instantly understandable and actually say what they actually are. In particular, I like to avoid acronyms as to be honest there is no need to make a variable or field name into an acronym and it is kind of lazy. Even small things like m_fsm for finite state machines should be m_finiteStateMachine.

Word choice for functions

I used to be bad for this. When naming functions make sure you represent how much work they are doing with your wording. For example, lets take a function called GetShipAttackPower(). What would you expect it to do? I would just expect it to access the data and grab that value out of the data. However when you open up the function you find it is is actually taking a variety of data and figuring out how much attack power the ship has. This is named wrong. This should be called calculate. Lets look at some code examples to explain this further:

public string GetShipName()
{
    return m_shipName;
}

The above Get function is only getting data, no calculation or look up work is being done. And in fact in this case, I would probably have very few, if any, of these functions. I will explain why later.

public int CalculateShipAttackPower()
{
    int attackPower = m_weaponAttack * m_weaponAttackPower;
    return attackPower;
}

The above is Calculating the attack power, not just getting it and so the function should be named to do so.

public int FindShipAttack(string shipID)
{
    ShipData shipData = Array.Find<ShipData>(m_allShipData, data => data.ID == shipID);
    Assert(shipData != null, "Could not find ship data for: " + shipID);
    return shipData;  
}

The above is not just Getting the data, work is actually been done to Find that data and thus the function name

Use of Properties

I really like properties, but they should be used in an intelligent manner. Here are some example of ways I like using properties:

public int Health { private set; get;}
public int AttackPower { private set; get; }

public EnemyShipData(int health, int attackPower)
{
    Health = health;
    AttackPower = attackPower;
}

If you are going to set a property in your constructor or an initialize method, instead of having a private member variable and a property that returns the member variable, I like to use the above.

In all other cases I like to do it the more standard way:

private int m_health = 100;
public in Health { get { return m_health; }}

What should NOT be done is this:

public int AttackPower
{
    get
    {
        return m_baseAttackPower * m_attackModifier;
    }
}

This is hiding logic and is a calculate function. Properties in my opinion should only be used for getting values. The Get property inside the property is not called calculate so why should it perform a calculate inside it?

Use of Singletons

Singletons are often used too much and break a lot of architectural patterns and can be seen as lazy. On my personal projects however, I have a rule of only having one or two which are extremely specific. In "Project Winter" for example I have 2: A GameController which looks after things like the game state and the game database which contains all the data. I am still deciding if this is the best way to do things, but for now this is how I use them. It is far from perfect and in the next year or so I may find a much better way than Singletons, however in my opinion, there has to be a good reason for them to exist and should be kept to a minimum.

Data lives in their own classes and files

Essentially, anything that is data lives in it's own class or struct (depending how they are passed around, etc). For example, in my rewrite of Shiro there will be an enemy fighter with a class where the data is stored and a controller class the data is passed to or is accessed. I really like this separation and makes things a lot neater in my opinion.

Lots of small specific classes and functions and avoid REGIONS

I find huge functions unreadable and I find code a lot easier to follow when it is split up into specific methods. It is a similar situation to classes. A class' contents should be specific and not contain a lot of "generic" functionality. If you find yourself writing out a load of regions to combat this, DON'T. Split the code up into several classes. It makes things a lot more readable and easier to follow.

Method Calls are laid out like a "Waterfall"

Essentially what I mean by this is that methods are laid in code out in the order they are called, or where they are called from. I hate having to jump up and down code files to go to method definitions. Here is an example of how I like to lay out my Methods

public bool FindIfShipAliveInCurrentLevelData(string shipID)
{
     bool result = false;
     int count = m_totalShipCount;
     for(int i = 0; i < count; ++i)
     {
         ShipData shipData = m_allShips[i];
         bool hasCorrectID = ShipHasID(shipData);
         if(hasCorrectID)
         {
              bool shipIsInCurrentLevel = IsShipInCurrentLevel(shipData);
              if(shipIsInCurrentLevel)
              {
                  bool shipIsAlive = IsShipAlive(shipData)
                  if(shipIsAlive)
                  {
                       result = true;
                        break;
                  }
              }
         }
     }
     return result;
}

public bool ShipHasID(ShipData shipData)...

public bool IsShipInCurrentLevel(ShipData shipData)...

public bool IsShipAlive(ShipData shipData)...

Now obvious refactoring of the above code aside (too many nested ifs), you can see that the methods are laid out after the function in the order they are called. They are all grouped together nicely, easy to find making the code file more readable.

Use of lambdas

I like use of Lambdas. I like to create extension methods that use them like my ForEvery which is a standard for loop wrapped in a lamda. However Lambdas need to be short (one line if possible to be readable)

For example this is a nice lambda expression:

m_allShips.ForEvery(ship => CheckIfShipIsDead(ship))

Where as this is too long:

m_allShips.ForEvery(ship =>
    {
         if(ship.Health <= 0)
         {
             ship.SetShipState(ShipState.Dead)
         }
    }

Although the second is only a couple of lines longer, this to me still looks messy.

Use of Asserts

Use them everywhere and liberally, make them tell you exactly what is wrong. You can have them be compiled out in production builds, but they make debugging a hell of a lot easier and make the program fail hard and fast so you know about it.

Use of braces on if and switch statements

I hate stuff like this

if(shipIsAlive)
   DoShipLogic()

Not adding braces there is pure and simply lazy. I always have braces around my if statements even if they are one liners.

The same with enums

I find reading this

switch(shipState)
{
   case ShipState.Alive:
      ShipAliveLogic();
      break;

   case ShipState.Dead:
      ShipDeadLogic();
      break;
}

Much harder than reading this

switch(shipState)
{
   case ShipState.Alive:
   {
      ShipAliveLogic();
      break;
   }

   case ShipState.Dead:
   {
      ShipDeadLogic();
      break;
    }
}

Use Conditionals

I really don't like the old school style #If #Elif #Else statements that are seen for compilation dependent code. I think they are messy, and in C# there are better ways to do them. That is where Conditionals come in

[Conditional("DEBUG")]
public void Assert(bool conditionIsTrue, string message)
{
    if(!conditionIsTrue)
    {
        throw new Exception(message);
    }
}

By adding the Conditional attribute to my Assert method that will now only be compiled into the program when the DEBUG flag is active.  lets look at a better example. Lets take some multiplatform code:

public void SaveGame()
{ 
#if WINDOWS
    WindowsSave();
#elif WII_U
    WiiUSave();
#elif PS_VITA
    PSVitaSave();
#endif
}

This can be turned into a nicer more readable format using Conditionals:

public void SaveGame()
{ 
    WindowsSave();
    WiiUSave();
    PSVitaSave();
}

[Conditional("WINDOWS")]
private void WindowsSave()...

[Conditional("WIIU")]
private void WiiUSave()...

[Conditional("PS_VITA")]
private void PSVitaSave()...

Sure it is more code, but it is more readable code.