HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Wa-Tor simulation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
torsimulationstackoverflow

Problem

I tried this as a programming exercise. It's working as intended, but I feel like the code is a mess and I could have used the advantages of object oriented programming much more (like inheritance for fish and shark from the Cell class, and probably much more). Can someone guide me a bit through this code example and show me how to make this more object-oriented?

The code simulates an ocean populated by fish and sharks:

  • Sharks hunt fish



  • Fish swim and breed



  • Sharks die of starvation after x years



The ocean, fish and sharks are all represented by a Cell class. The objects are stored in a 2D array and then painted onto the Windows form.

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Diagnostics;
using System.Threading;

namespace WaTor
{
public partial class Form1 : Form
{

//Gritsize width(x) and height(y)
const int TableX = 10;
const int TableY = 10;
const int pixelSizeX = 19;
const int pixelSizeY = 19;

//Fish Properties
int initialNumberOfFish = 2;
int initialNumberOfSharks = 2;
int ageToBreedFish = 5;
int ageToBreedShark = 10;
int yearToStarvation = 5;

int sharkScent = 3;

Random rnd = new Random();

Cell[,] myArray = new Cell[TableX, TableY];

public Form1()
{
InitializeComponent();
intitializeArray();
}

private void intitializeArray()
{

int x = 0; //Position on X-Axis
int y = 0; //Position on Y-Axis
for (int i = 0; i yearToStarvation)
{
kill(i, j);
Debug.WriteLine("Hai verhungert");
}

//If not starved shark hunts
hunt(i, j);

}

else
//If Cell is Fish and hasent moved

Solution

Well, first of all I would suggest you separate your concerns a little more. At the moment you have (most of) your Wa-Tor logic AND your Windows Forms code all together in the same class. Secondly, that class is named Form1! I know Visual Studio is nice when it does things for you, but naming classes isn't it's strong-suit ;)

Separate your presentation from your (domain) logic

As you get into more complex problem spaces, things like decomposition and domain modelling become important steps in translating the problem space into abstractions that you can compose together.
Imagine if your Wa-Tor simulation was actually being run on a super computer, in order to produce an accurate model of sea-life populations and breeding patterns in the Pacific Ocean. This would obviously be a massive increase in scope, but shouldn't require any particularly large code changes* (known as refactoring) if your code has been written in a re-usable, decomposed, loosely-coupled way (which is the goal of all OOP coders).

* well, assuming the logic of Wa-Tor is exactly how the Ocean works.

I would suggest that things such as initialNumberOfFish,
initialNumberOfSharks, ageToBreedFish, ageToBreedShark & yearToStarvation, etc all belong somewhere other than the (presentation) class Form1. The same goes for the storage, size and initialization (intitializeArray) of your array (a better name for which might be "world", or ocean, etc). That class should probably be something along the lines of Simulation, World, etc.

The code in intitializeArray can go into the constructor of that Simulation class. That constructor can also accept all of the configuration parameters of the simulation (size, age to breed number of fish, etc) as arguments. It can also then call the populate method.

Also, an additional problem of your presentation code infecting your logical (model) code.. Why does a Cell have a cellColor? Colors are something pertaining to display, the Cell may have a state, status, type or even an isEmpty property - as these all relate to the simulation (which is what the Cell is part of modelling).

Abstract your behaviours as well as your representations...

  • OR : "Don't repeat yourself!"



As @apieceoffruit commented, the Single Responsibility Principle (closely related to the Separation of Concerns I mentioned earlier) should be employed when you are deciding upon the responsibilities for your objects (often called "Don't Repeat Yourself", or DRY). Since objects contain both data (i.e. fields) and behaviour (methods), this principle should be used (as @apieceoffruit said) when designing your methods.

In particular, the populate method, contains the following code:

//Distribute Fish randomly
    int PosX=0;
    int PosY=0;
    bool isEmpty=false;
    Random rndX = new Random();
    Random rndY = new Random();
    PosX = rndX.Next(0, TableX);
    for (int i = 0; i < initialNumberOfFish; i++)
    {
        while (!isEmpty)
        {
            PosX = rndX.Next(TableX);
            PosY = rndY.Next(TableY);
            if (myArray[PosX, PosY].fishType == fishTypeEnum.empty)
                isEmpty = true;
        }
        myArray[PosX, PosY].fishType = fishTypeEnum.fish;
        myArray[PosX, PosY].cellColor = Color.Green;
        myArray[PosX, PosY].age = rndX.Next(0, ageToBreedFish);
        isEmpty = false;
    }


Which is then almost copied verbatim for "Distribute Sharks randomly" (obviously with fishTypeEnum.shark used instead, etc).

This could (and would) be much better Decomposed into a helper method DistributeRandomly, or (as @apieceoffruit commented) the two methods: DistributeFish & DistributeSharks (hopefully which could share some common base helper method, since they're so similar).

There also seems to be something very similar in fishNearBy, which has four near identical for-loops. The may be able to be abstracted out into a helper method that fishNearBy can call 4 times, thus saving on code and making refactoring of that logic easier in the future.

Inheritance: The Answer to All your Problems   (NOT)

As you mentioned in your question, "inheritance for fish and shark from the cell class", might be a good idea. It depends if your behaviours can be made generic enough to apply to both.

For example, in the Wikipedia page about Wa-tor^, it is stated:


Since hunting for fish takes priority over mere movement, the rules for a shark are more complicated: from the adjacent points occupied by fish, select one at random, move there and devour the fish. If no fish are in the neighborhood, the shark moves just as a fish does, avoiding its fellow sharks.


(Emphasis added)

^ Wikipedia attributes this (in a quotation) to Dewdney (1984) "Sharks and fish Wage an ecological War on the toroidal planet Wa-Tor", Scientific American December pg I4—22

From reading this, I would have (as an OOP programmer naturally would,) thought that the move behavio

Code Snippets

//Distribute Fish randomly
    int PosX=0;
    int PosY=0;
    bool isEmpty=false;
    Random rndX = new Random();
    Random rndY = new Random();
    PosX = rndX.Next(0, TableX);
    for (int i = 0; i < initialNumberOfFish; i++)
    {
        while (!isEmpty)
        {
            PosX = rndX.Next(TableX);
            PosY = rndY.Next(TableY);
            if (myArray[PosX, PosY].fishType == fishTypeEnum.empty)
                isEmpty = true;
        }
        myArray[PosX, PosY].fishType = fishTypeEnum.fish;
        myArray[PosX, PosY].cellColor = Color.Green;
        myArray[PosX, PosY].age = rndX.Next(0, ageToBreedFish);
        isEmpty = false;
    }

Context

StackExchange Code Review Q#118992, answer score: 3

Revisions (0)

No revisions yet.