Coding Guide

From WCell Wiki
Jump to: navigation, search

Having a clean codebase is fundamental to the health of any project, be it open- or closed-source, hobby or professional. We all are human and we all know: Errare humanum est - To err is human. And because of that the WCell team started composing this guide which includes: Guidelines for all developers, special guidelines for C# developers, as well as a few guidelines that only apply to WCell. The WCell team (as well as any willing contributor) will add more useful hints along with the absolute no-nos of coding over time.

NOTE: Examples are prefixed with an arrow "->"

General Coding Guidelines

  • Do null-checks for all variables that might be null unless you are 100% sure that they cannot be null in the given context (eg this (if within instance scope) can never be null)
    • When writing methods that allow for null - arguments OR never return null, mention it in the documentation of that method
    • When calling other methods, make sure that the arguments are not null, unless they have been passed into this method OR the documentation states explicitely that arguments are allowed to be null. This way further null-checks can be avoided and your code stays safe
    • If required parameters are null, you should not need to check for null or throw an ArgumentNullException, since following the rules above would ensure that this cannot happen
      • -> This is a no-no:
      var x = SomeMethod(xx).SomeMemberOfTheReturnValue;
unless it is given that SomeMethod would never return null (in which case it should be mentioned in the methods' documentation)
  • Don't spread code that belongs to one system all over the place. Always keep it in that system
  • If the relationship between 2 systems is defined through more than one or two methods/fields, consider creating a class (or set of classes) to represent the relationship between the 2 systems
  • Naming methods, fields and properties correctly and putting them in the right place is fundamental! Always make sure that a method is where it should be and wether it has the right name and parameters to ensure that it is most re-usable!
    • When using OOP: Try to keep static methods and global functions to a minimum. Consider their parameters and decide if you can:
    • Move the method to one of the parameter-types OR:
    • Move the method to a new class that represents the relationship between (some of) the parameter-types
      • -> QuestMgr.ShareQuest(Character, Quest) can most certainly be moved to Quest which represents the progress of a Character within a specific Quest
      • -> QuestMgr.CanAcceptQuest(Character, QuestTemplate) can most certainly be moved to QuestLog which defines the relationship between a Character and all its Quests
  • If you have a list whose size might exceed 10 and you need to call Contains(obj) or Remove(obj) on it regularly, you usually should use HashSet<T> (or something equivalent in the language of choice) instead:
    • Lookup of existing objects in HashSets is very fast - similarly to Dictionaries it has a Complexity class of O(1) (against O(n) for a list)
    • Since a HashSet doesn't use indices, it doesnt have Remove(index) or other methods that use indices
  • When you need a collection to map by integral keys (number-keys), consider using an Array over using a Dictionary, if you can ensure that resizing of the array will be reduced to a minimum and there won't be a lot of big gaps between the used indices
    • Simple Arrays (not Associative Arrays) are still the fastest collection in most languages
    • Most collections are backed up by arrays (in C#: List<T>, HashSet<T> and Dictionary<K, V> are, as well as many others)
    • Arrays are static in size but they can be resized (which is what collections that are backed up by arrays do)
    • WCell provides Extension- and Utility- methods through Cell.Core.ArrayUtils - Those are helpful when you work with indices that might be out of Array's bounds
  • If possible, don't implement Method(id) but implement Method(objWithId) instead: If you want to use the method later in another context where you already looked up the object, you don't want the method to look up the object by its id again
  • If class A holds several array-fields that all map certain values by a unified index, rather create a new class B that holds all values of those arrays and make class A have a field that holds an array of B's
  • If you see a major mistake in your code's logic, remember: Its never too late for Refactoring
    • If you don't refactor it now, you will hate it until the end and maybe until it bothers you too much to continue working on that code anymore at all
    • By refactoring a lot, you also learn a lot and get more and more used to writing a clean codebase
  • When writing a server application, always validate all client input in the PacketHandlers - Keep in mind that a cheater can send *anything* he/she wants

C# Specific Guidelines

  • Keep methods or properties, that somehow have something to do with each other, together and consider enclosing them with a #region block
  • Don't use too many #region blocks and if a class is not getting too huge, don't nest them
  • Consider seperating a single class into several files if you can make a logical seperation of the functionality that each file of the class contains


Naming Guidelines

  • Keep names short and don't try to put every bit of information into it
  • Always stay in the same language (preferred english due to the given english symbol names of every programming language)
  • Don't add fill-words (just etc), the name of the symbol type (Class, Method, Field, Attribute etc) or grammatical sugar (articles, prepositions, conjugations) unless it makes perfect sense
  • Be careful when abbreviating:
    • Stick to commonly understood abbreviations when naming global symbols
    • If you use custom or uncommon abbreviations, explain in the documentation of the symbol
    • Don't use one- or two-letter names unless its common practice or it makes enough sense:
      • -> i,j,k etc are commonly used for constantly changing local integers in loops
  • Cases:
    • The case to be used in naming is usually never up to the developers' choice. To keep things in order, every developer is always recommended to stick to the naming that the language is using by default
    • In C# we always use PascalCase for all publicly accessible members of a class or namespace and camelCase for all local variables
    • -> var myLocalVariable = someObj.SomePublicProperty;
    • -> class X { public int Y; }

Code understanding and Documentation

Code should always be as self-explanatory as possible, which means that by just chosing the right names for every symbol (variable, method, class, event, delegate etc), other developers should be able to figure out what each symbol does. But there is a limit to what one can express through names. That is why one needs Documentation to compensate for missing information:

  • Documentation should always be short and precise
  • Documentation should especially explain anything that the name itself does not reveal, eg:
    • Required context: Especially in multi-threaded environments, certain methods must only be called from certain Threads (see WCell's Threading Strategy)
    • Argument restrictions: If an argument may be null or may only have a very limited range, make sure to mention that (if you are not able to ensure that through encapsulation)
    • References to other symbols that can be used instead of this one, eg:
      • WCell.RealmServer.Character.Save should reference WCell.RealmServer.Character.SaveLater and vice versa

WCell Guidelines

  • WCell is trying to seperate Core (the Model) from Input and Output (similar to the MVC pattern), thus Input and Output should be seperated from the "model" or "core":
    • Send - methods (methods that send packets), as well as PacketHandlers (methods that handle client input) should usually be defined in a *Handler class in the "Handlers/" - folder and should not contain any validation or evaluation of the given data
    • Don't call Send* - methods directly from PacketHandlers unless there is really no validation needed (which is almost never the case)

Hints when developing with VS

  • Ctrl+R, Ctrl+R (yes, twice) is the shortcut to rename the symbol that the cursor is currently on
  • F12 to jump to the definition of the symbol that the cursor is currently on
  • Shift+F12 to lookup all references of the symbol that the cursor is currently on
  • Ctrl+F = Search, Ctrl+Shift+F = Search All (displays results in a list), Ctrl+R = Replace, Ctrl+Shift+R = Replace All (displays results in a list)
  • Resharper gives the developer a huge boost of comfort and tons of much needed features when writing code

More

  • See the Development section for all Development-related articles
  • Read up on why a Clean Codebase is so important