1. We have added the ability to collapse/expand forum categories and widgets on forum home.
    Dismiss Notice
  2. All Civ avatars are brought back and available for selection in the Avatar Gallery! There are 945 avatars total.
    Dismiss Notice
  3. To make the site more secure, we have installed SSL certificates and enabled HTTPS for both the main site and forums.
    Dismiss Notice
  4. Civ6 is released! Order now! (Amazon US | Amazon UK | Amazon CA | Amazon DE | Amazon FR)
    Dismiss Notice
  5. Dismiss Notice
  6. Forum account upgrades are available for ad-free browsing.
    Dismiss Notice

Writing Readable Code

Discussion in 'Civ5 - Modding Tutorials & Reference' started by Thalassicus, Feb 2, 2012.

  1. Thalassicus

    Thalassicus Bytes and Nibblers

    Joined:
    Nov 9, 2005
    Messages:
    11,057
    Location:
    Texas
    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.
    Code:
    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:

    Code:
    ((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.
    PHP:
    function GetPlotsInCircle(plotminRadiusmaxRadius)
        
    local plotList    = {}
        
    local mapWmapH  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(0centerX-maxRadiusmapW-1)
        
    x2 isWrapX and ((centerX+maxRadius) % mapW) or Constrain(0centerX+maxRadiusmapW-1)
        
    y1 isWrapY and ((centerY-maxRadius) % mapH) or Constrain(0centerY-maxRadiusmapH-1)
        
    y2 isWrapY and ((centerY+maxRadius) % mapH) or Constrain(0centerY+maxRadiusmapH-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(xy)
        while (
    yStep rectH) and adjPlot ~= nil do
            while (
    xStep rectW) and adjPlot ~= nil do
                if 
    IsBetween(minRadiusMap.PlotDistance(xycenterXcenterY), maxRadiusthen
                    table
    .insert(plotListadjPlot)
                
    end
                
                x        
    1
                x        
    isWrapX and (mapW) or x
                xStep    
    xStep 1
                adjPlot  
    Map.GetPlot(xy)
            
    end
            x        
    x1
            y        
    1
            y        
    isWrapY and (mapH) or y
            xStep    
    0
            yStep    
    yStep 1
            adjPlot  
    Map.GetPlot(xy)
        
    end
        
        
    return plotList
    end
     
  2. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,585
    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.
     
  3. The_J

    The_J Say No 2 Net Validations Retired Moderator

    Joined:
    Oct 22, 2008
    Messages:
    29,759
    Location:
    Germany / Netherlands
    :thumbsup: 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)
     
  4. Thalassicus

    Thalassicus Bytes and Nibblers

    Joined:
    Nov 9, 2005
    Messages:
    11,057
    Location:
    Texas
    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:

    PHP:
    x1 isWrapX and ((centerX-maxRadius) % mapW) or Constrain(0centerX-maxRadiusmapW-1)
    x2 isWrapX and ((centerX+maxRadius) % mapW) or Constrain(0centerX+maxRadiusmapW-1)
    y1 isWrapY and ((centerY-maxRadius) % mapH) or Constrain(0centerY-maxRadiusmapH-1)
    y2 isWrapY and ((centerY+maxRadius) % mapH) or Constrain(0centerY+maxRadiusmapH-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:

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

    if isWrapY then
      y1 
    = (centerY-maxRadius) % mapH
      y2 
    = (centerY+maxRadius) % mapH
    else
      
    y1 Constrain(0centerY-maxRadiusmapH-1)
      
    y2 Constrain(0centerY+maxRadiusmapH-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.
     
  5. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,585
    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)
     

Share This Page