Code reuse considered harmful

The title is intended to call for attention. This post is about one perspective of software development in the light of my own experience in the area, it won’t contain anything really revealing and is not to be taken as an absolute true for life. It’s a rant. I hope you have a good time reading it, feel free to leave me any kind of feedback.

I see a bunch of people praising reuse as being the prime thing of good software development, and few talking about replaceability. There seems to be a constant seek to avoid writing code that is used only once, as if it was a really bad thing. Then we end up with software that is made of conceptual factories that create factories that create the things the software really needs, yes there are two levels of factories, or more. Is this really necessary? How much time do we save by this extreme look for reusing code?

First, let me ask and answer a simple question: why duplicated code is annoying? Well, duplicated code makes it harder to change stuff. When you have the same piece of code written multiple times in a code base and you find that it needs a change, e.g. bug fix or new feature, you will need to change it in all places. Things can get worst if you don’t know all places where the code is duplicated, so you may forget to change one of these spots. The result is that duplicated code is a sign of harder maintenance and a fertile ground for further bugs to spawn. That’s why we learned to hate it. We started fighting this anti-pattern with all strength we had.

Code reuse is the perfect counter to code duplication, right? Sure, it is right, if we reuse a piece of code in two places, we have no duplication between these places. So, we did it! We found the Holy Grail of code quality, no more duplicated code, yay! But something unintended happened. Remember the old saying: with great powers, comes great responsibility. People started to be obsessed with it. As soon as they learned to use the hammer of code reuse, everything turned into a nail, when it didn’t work out in the first hit, they adjust the size of the hammer and hit it again with more effort.

This seek after code reuse led us to a plethora of abstractions that seems to handle every problem by reusing some code. Don’t get me wrong, lots of them are useful, these are the ones that were created from observation. The problem is the ones that are created from “it’s cool to abstract”, or other random reason that is not true observation. We see frameworks after frameworks that try to fit every problem of the world into a single model. Developers learn to use these frameworks and suddenly find out that the framework creator is wrong and create yet another abstraction over it or creates yet another framework that tries to use a different model to solve the world.

What happens when we have a bug in one of these abstractions or we need to enhance it? Silence, for a while, then the sky turns black, you take a break, go for a walk, come back to your computer and start blaming the other developer that created the bug or that “got the abstraction wrong”, because your vision was the right one. What happened? We reused code to avoid code duplication, but we are still having the same problems: code that is hard to maintain and evolve.

My guess? We missed the enemy. Code duplication is not our enemy. Maintenance problem and rigidity of code is.

My tip? Give more focus on replaceability of code instead of reuse in your talks, codes, classes, etc. Create the right abstraction to fix the problem at hand in a way that is easy to replace the underlying code when needed. Some time in the future, you will need to change it anyway. That’s what agile methodologies try to teach us: embrace change. Planning for a design to be reused says: “my design will be so awesome, that I will reuse it everywhere.” That’s what agile says: “your design will need to change sometime, because the requirements will change, plan for the replaceability of it.” People are doing things like service oriented architecture in the wrong way because they are looking for reuse of services and not for replaceability of services, they end up with a Big Web of Mud.

That’s all folks. Thanks for your time.

Unity3D, Bejeweled and Domain-driven design

I’m working in a new game like Bejeweled. I’m happy with the freedom of code organization that Unity3D engine allows. During my first contacts with it, I thought that almost everything would be oriented to MonoBehaviour class, but this showed to be false. This class is necessary just as a glue point between any C# code and the objects of the engine. I’ll report how I’ve started coding this game and the changes I made so far, you can watch the following video to see the current state of the game:

I started creating a GameObject for every entity that I identified in the game mechanics:

  1. Board
  2. Piece

The board contains every pieces and manages them:

public class Board : MonoBehaviour {
  private GameObject[,] pieces;
  void Awake() {
    pieces = /* Initialize pieces */;
  }
}

The piece type is defined by a MonoBehaviour that exposes an enumeration:

public class Piece : MonoBehaviour {
  public PieceType Type;
}
public enum PieceType {
  Circle,
  Square,
  Star,
  Triangle,
  Hexagon,
  Polygon
}

After the definition of the entities participating in the game, I started to code game’s logic inside these classes. It worked for a while, but some problems appeared. The same classes had lots of different responsibilities (i.e. game’s rules, animations, handling input) and this made it hard to code some stuff, because I needed to maintain a mind map of all concerns to avoid breaking something. Also, during animations, the board in memory was in an inconsistent state, waiting for the end of the animation to then continue processing.

Recently I’ve read some stuff about Domain-driven design (DDD) and decided to apply a bit of it in this game. My first step was to separate my code domain from the others, I selected game’s mechanics as my core domain: if this part is not well behaving and it’s hard to maintain, I’ll be in a bad spot. Then I went to create this domain classes completely separated from the rest of the game, I ignored the existence of Unity3D at this point.

I only seen a single entity for this domain: the board. It makes no sense for the piece to exist on its own, everything that involves pieces always happens inside the board. I still have a class for the piece, but it is an internal thing of the board. My design became this:

public class BoardPosition {
  public readonly int Row;
  public readonly int Column;
  public BoardPosition(int row, int column) {
    Row = row;
    Column = column;
  }
}

public class Board {
  private Piece[,] pieces;
  public Board() {
    pieces = /* Initialize pieces */;
  }

#region Queries
  public Piece PieceAt(BoardPosition p) { /* ... */ }
#endregion

#region Events
  public delegate void PieceCreatedDelegate(BoardPosition position, Piece piece);
  public event PieceCreatedDelegate PieceCreated;

  public delegate void PieceDestroyedDelegate(BoardPosition position);
  public event PieceDestroyedDelegate PieceDestroyed;

  public delegate void PieceMovedDelegate(BoardPosition from, BoardPosition to);
  public event PieceMovedDelegate PieceMoved;

  public delegate void PiecesSwappedDelegate(BoardPosition a, BoardPosition b);
  public event PiecesSwappedDelegate PiecesSwapped;
#endregion

#region Commands
  public void SwapPieces(BoardPosition a, BoardPosition b) {
    ...; // Swap pieces
    PiecesSwapped(a, b);
  }

  public void StepGameState() {
    ...; // Destroy pieces
    ...; // Move pieces
    ...; // Create pieces

    for (...) {
      PieceDestroyed(...);
    }
    for (...) {
      PieceMoved(...);
    }
    for (...) {
      PieceCreated(...);
    }
  }
#endregion
}

This way, the view part of the game register itself to handle the events generated by the board and update the user interface as needed.

public class BoardView : MonoBehaviour {
  private Board board;
  private GameObject[,] pieces;
  void Awake() {
    board = new Board();
    board.PieceCreated += HandlePieceCreated;
    board.PieceDestroyed += HandlePieceDestroyed;
    board.PieceMoved += HandlePieceMoved;
    board.PiecesSwapped += HandlePiecesSwapped;
    pieces = /* Initialize pieces based on 'board' */;
  }

  public void HandlePieceCreated(BoardPosition position, Piece piece) { /* ... */ }
  public void HandlePieceDestroyed(BoardPosition position) { /* ... */ }
  public void HandlePieceMoved(BoardPosition from, BoardPosition to) { /* ... */ }
  public void HandlePiecesSwapped(BoardPosition a, BoardPosition b) { /* ... */ }

  void Update() {
    board.Step();
    if (/* ... */) {
      board.SwapPieces(a, b);
    }
  }
}

This design made it hard to sync time between the model and the view. The model calls the methods of the view to notify about changes, the view has little space left to decide when to handle each event. In my case, some events started animations that needed to hold other events from happening, i.e. there is a temporal sequencing between some events.

I changed the model to return a list of events that happened at each command, instead of calling the handler directly:

#region Events
public interface BoardEvent {}
public class PieceCreated : BoardEvent { /* ... */ }
public class PieceDestroyed : BoardEvent { /* ... */ }
public class PieceMoved : BoardEvent { /* ... */ }
public class PiecesSwapped : BoardEvent { /* ... */ }
#endregion

#region Commands
public List SwapPieces(BoardPosition a, BoardPosition b) { /* ... */ }
public List StepGameState() { /* ... */ }
#endregion

Now, the view needs to call the handlers itself, but can decide when to handle each event:

public class BoardView : MonoBehaviour {
  private List events;
  void Update() {
    if (events.Count < 1) { events = board.StepGameState(); }
    foreach (BoardEvent e in events) {
      if (CanHandleNow(e)) {
        Handle(e);
      }
    }
    // ...
    if (HandledEverything) { events.Clear(); }
  }
}

After this, I still felt that this temporal sequencing was not clear, it was “floating in the air”. I decided to put it into the model, it’s part of my domain: every event has a temporal identifier:

public class Board {
  private int timeCounter;
  public List StepGameState() {
    ...; // Destroy pieces
    for (...) {
      events.add(new PieceDestroyed(timeCounter, ...));
    }
    if (eventHappened) { timeCounter++; }

    ...; // Move pieces
    for (...) {
      events.add(new PieceMoved(timeCounter, ...));
    }
    if (eventHappened) { timeCounter++; }

    ...; // Create pieces
    for (...) {
      events.add(new PieceCreated(timeCounter, ...));
    }
    if (eventHappened) { timeCounter++; }

    return events;
  }
}

public class BoardView : MonoBehaviour {
  private int timeCounter;
  private List events;
  void Update() {
    if (events.Count < 1) { events = board.StepGameState(); }
    foreach (BoardEvent e in events) {
      if (e.When() == timeCounter) Handle(e);
      if (e.When() > timeCounter) {
        stillHasEventsToHandle = true;
        break;
      }
    }
    if (/*handledAnimationOfAllEventsOfMyTimeCounter*/) { 
      // Advance time perception of view
      timeCounter++;
    }
    if (!stillHasEventsToHandle) {
      events.Clear(); // Will step game state at next frame
    }
  }
}

Both view and model has a temporal identifier and the sync is more evident.

The actual code is looking very similar to the listed here. The model is handling well up to now. I feel bad about one thing: the Step command of the model may leave the board in a “not-consolidated” state, as it makes a single interaction to check for matching groups to be removed from the board. The view then needs to call the Step command more than once between handling two inputs from the user. I didn’t want to make a lot of interactions in a single Step to avoid putting lots of stuff in memory before anything is handled by the interface, looks like a waste to me. I miss the lazy part of Haskell.

I still have lots of stuff to add to the game’s mechanics (my core domain). I’ll see the problems of this design in the next days and will post news with the next changes. Critics and suggestions are welcome.