kkisiele / codekata Goto Github PK

You asked for code reviews on the telegram chat so here are my two cents:

Any object should have an interface. For instance, in your minesweeper game, you place the mines on the Field after the Field is created. Instead, I believe Field should be an interface with a few implementations, and then when you create each Field, you wrap them with decorators. Some of them will be wrapped in the Mined decorator. For instance:

Field mined = new Mined (
    new Hidden()
);
mined.uncover(); //boom, inside Mined.uncover(), you throw an exception.

Field clear = new Hidden(
    new Number(1);//or 2, or 3, or make this dynamic based on the table's size.
);

clear.uncover();//will uncover it just fine since it is not wrapped inside Mined decorator. 

//Hidden.uncover() may look like this, taking into account that classes should also be immutable
public Field uncover() {
    new Visible(this);
}

Field is an interface, while Hidden, Number, Mined and Visible are all implementations of it. I think with such a design, you wouldn't even need FieldState anymore.

Furthermore, uncover() may actually return Field[] since, in Minesweeper, when you uncover a Field, sometimes more neighbouring fields are uncovered together, simultaneously.