Writing Readable Code

[Thalassicus]

There's a basic tradeoff in programming:



As processor power rises and memory prices drop, the bottom two aspects of programming become less important. It's increasingly vital to create code that's easy to write, read, and maintain. I'm going to discuss some basics of the human factor here.

1. Why?
2. Guidelines
3. Further Details
4. Example

Why?​

You should be able to come back to your work some time later and still understand what the heck you did. If your code looks like gibberish, you waste time figuring it out again.

For example, I've copied below some real code from a popular Civ 5 mod. Code like this is difficult to read. The variables have cryptic names, which are hard to understand and difficult to search for with find-replace tools. The formatting is also inconsistent. Some "if" statements are on multiple lines, while others are on one line. Writing code better than this makes it easier for you (and fellow modders!) to use or alter the code later.

if cT ~= "" then
  repeat
  local t1, t2; local more = false;
  s, e, c, d = findToken( cT, e );
  if s ~= nil then t1 = deserialize( c ); end
  if d == "=" then
    s, e, c, d = findToken( cT, e+2 );
    if s ~= nil then t2 = deserialize( c ); end
  end
  if d == "," then e = e+2; more = true; end
  if t2 ~= nil then r[t1] = t2;
  else table.insert( r, t1 );
  end
  until e >= len and not more;
end

Guidelines​

These are two important aspects of writing readable code:

1. Use good formatting, with spaces and indentation. The earlier code example is cramped and hard to read.
2. Give variables descriptive names so they are instantly recognizable.

When formatting your code, it's a good practice to write code blocks like if-then-else statements on separate lines. Add extra lines between important segments of code. When writing complicated statements with multiple nested parenthesis, vary your spacing so it's easy to read like the example below. Inner variables are grouped together (centerY+maxRadius) while outer variables are spaced out:

((centerY+maxRadius) % mapH) or Constrain(0, centerY+maxRadius, mapH-1)

The common naming standard in the game industry is camel case:

SomeFunctionName()
someVariableName
CONSTANT_VALUE

Join words (without spaces) into the name. Capitalize the first letter of functions, while leaving the first letter of variables lowercase. Constants are all caps with an _ underscore separating words.

Variable names are the most obvious part of Civ 5's code with poor design. Some parts of the code use camel case, while others use a variant called Hungarian notation. Pick one style and stick to it! This inconsistency makes it difficult to work with the vanilla Firaxis code. I'd recommend avoiding Hungarian notation in Lua/C++ for the following reasons:

Spoiler :

Hungarian notation is a naming convention in which the name of a variable or function indicates its type or intended use.

• When names are sufficiently descriptive, the additional type information can be redundant. E.g. firstName is most likely a string. So naming it sFirstName only adds clutter to the code.
• If a variable's type is changed, either the decoration on the name of the variable will be inconsistent with the new type, or the variable's name must be changed.
• Modern integrated development environments display variable types on demand, and automatically flag operations which use incompatible types, making the notation largely obsolete.
• Compilers for languages providing type-checking ensure the usage of a variable is consistent with its type; checks by eye are redundant and subject to human error.

"Building the type of an object into names simply complicates and minimizes abstraction."
~ Bjarne Stroustrup, C++ creator

"Encoding the type of a function into the name (so-called Hungarian notation) is brain damaged—the compiler knows the types anyway and can check those, and it only confuses the programmer."
~ Linus Torvalds, Linux creator

When you read your code out-loud it should somewhat make sense. Variable names are usually a noun, and function names are a verb. These are the most common function name prefixes:

• Do : perform some action without a return value
• Is : returns a true/false condition
• Get : retrieve a value
• Set : store a value

Further Details
​

For further information about good coding practices, search Google for "coding standard" or check out the link below. Most standards of C++ also apply to similar languages like Lua and Java.

www.possibility.com/Cpp/CppCodingStandard.html

Example​

Below is an example of easily readible Civ 5 Lua code to get plots within a circular area. Variables names are descriptive, and tasks are separated with blank lines. The tasks are:

1. Initialize variables.
2. Check for world wrap.
3. Initialize variables dependent on world wrap.
4. Loop through each row and column of adjacent plots, and store plots with a distance between minRadius and maxRadius into plotList.

function GetPlotsInCircle(plot, minRadius, maxRadius)
    local plotList    = {}
    local mapW, mapH  = Map.GetGridSize()
    local isWrapX     = Map:IsWrapX()
    local isWrapY     = Map:IsWrapY()
    local centerX     = plot:GetX()
    local centerY     = plot:GetY()
    
    x1 = isWrapX and ((centerX-maxRadius) % mapW) or Constrain(0, centerX-maxRadius, mapW-1)
    x2 = isWrapX and ((centerX+maxRadius) % mapW) or Constrain(0, centerX+maxRadius, mapW-1)
    y1 = isWrapY and ((centerY-maxRadius) % mapH) or Constrain(0, centerY-maxRadius, mapH-1)
    y2 = isWrapY and ((centerY+maxRadius) % mapH) or Constrain(0, centerY+maxRadius, mapH-1)

    local x      = x1
    local y      = y1
    local xStep  = 0
    local yStep  = 0
    local rectW  = x2-x1 
    local rectH  = y2-y1
    
    if rectW < 0 then
        rectW = rectW + mapW
    end
    
    if rectH < 0 then
        rectH = rectH + mapH
    end
    
    local adjPlot = Map.GetPlot(x, y)
    while (yStep < 1 + rectH) and adjPlot ~= nil do
        while (xStep < 1 + rectW) and adjPlot ~= nil do
            if IsBetween(minRadius, Map.PlotDistance(x, y, centerX, centerY), maxRadius) then
                table.insert(plotList, adjPlot)
            end
            
            x        = x + 1
            x        = isWrapX and (x % mapW) or x
            xStep    = xStep + 1
            adjPlot  = Map.GetPlot(x, y)
        end
        x        = x1
        y        = y + 1
        y        = isWrapY and (y % mapH) or y
        xStep    = 0
        yStep    = yStep + 1
        adjPlot  = Map.GetPlot(x, y)
    end
    
    return plotList
end

 

[Pazyryk]

Great post! With 2000+ lines of Lua cod in my mod now, I'll add a few of my own:

Be consistent in your naming
Firaxis uses iPlayer / playerID and iBuilding / buildingID interchangeably, and then to add confusion they use "i" for things that aren't object IDs or table row IDs (following the Hungarian notation for integers, but not consistently). I use iUnit, iPlayer for game object IDs, and then buildingID, policyID, etc for table row IDs. All other variables get simple descriptive lower case. I'm not saying you should follow my rules. But it is worth the time to "formalize" your own before you get too far.

Don't pass functions around willy-nilly
Firaxis loves to do this. Not only does it make code incomprehensible to others, if you aren't very careful you will probably take a big performance hit (see item 8 here). Sure, sometimes it is the best solution to a problem. But think about alternatives.

Rules can be broken
Don't be too rigid. While I follow the normal separate line convention for if then blocks, there are several places in my code where I have dozens of lines like this:

if something then return false end
if something then return false end
if something then return false end
if something then return false end
if something then return false end
if something then return false end
if something then return false end
if something then return false end

That's less readable on separate lines. So bending rules sometimes helps, if there is a reason and the pattern is easy to see.

 

[The_J]

to that 2 posts, both well thought.

As small side mark: The hungarian notation is probably a left over from Civ4, where it was also used, probably due to Python as language, to which 3 of the 4 things in the spoiler do not apply.
(but that's nearly offtopic now)

 

[Thalassicus]

Rules can be broken
Don't be too rigid. While I follow the normal separate line convention for if then blocks, there are several places in my code where I have dozens of lines like this:

I agree completely about this and the other tips you posted. That's why I consider these "guidelines" instead of rules. For one-line if statements, particularly when assigning a variable, I use the ternary "x and y or z" statement like in the earlier example:

x1 = isWrapX and ((centerX-maxRadius) % mapW) or Constrain(0, centerX-maxRadius, mapW-1)
x2 = isWrapX and ((centerX+maxRadius) % mapW) or Constrain(0, centerX+maxRadius, mapW-1)
y1 = isWrapY and ((centerY-maxRadius) % mapH) or Constrain(0, centerY-maxRadius, mapH-1)
y2 = isWrapY and ((centerY+maxRadius) % mapH) or Constrain(0, centerY+maxRadius, mapH-1)

In this situation it's not possible to directly replace the and-or statement with if-then-else. The code has to be written in a lengthier format with each x1/y2/etc variable name repeated twice:

if isWrapX then
  x1 = (centerX-maxRadius) % mapW
  x2 = (centerX+maxRadius) % mapW
else
  x1 = Constrain(0, centerX-maxRadius, mapW-1)
  x2 = Constrain(0, centerX+maxRadius, mapW-1)
end

if isWrapY then
  y1 = (centerY-maxRadius) % mapH
  y2 = (centerY+maxRadius) % mapH
else
  y1 = Constrain(0, centerY-maxRadius, mapH-1)
  y2 = Constrain(0, centerY+maxRadius, mapH-1)
end

While both styles are valid, in this circumstance I find it easier to read the compact grid-like form. It's also a habit from using the ?: operator in c++/java. Click Here for more information about ternary statements in lua.

 

[Pazyryk]

I started with the Hungarian system (a holdover from my limited Civ4 modding). I spent about 5 hours cleaning it out of my code some 6 months ago. I can honestly say that in the 10000 errors I've made and fixed over the last 6 months, not a single one was related to confusion in variable type.

(though I will still use "bDisable"... but only because it is short and informative)