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

Memory with a twist

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

Problem

Description

This is the good old game Memory with a twist: Every time you pick a wrong pair, the two tiles you chose will switch their location. So sometimes you might think that a tile is at one location when in fact... it has moved. And it might feel like you have no idea where it is anymore.

Author of this game is not responsible for any broken keyboards, screens and/or mouse devices.

I am using a semi-Java8-compliant version of GWT. It does not support the Stream API.

Where to play?

The game can be played here: http://www.zomis.net/codereview/memory/MemoryGWT.html

Class Summary

  • MemoryGWT.java, MemoryGWT.html: Main GWT Entry Point.



  • MemoryGWT.css: Just some simple CSS.



  • MemoryBoard.java: The view class used for the main Memory Board



  • FieldView.java: The view for each tile.



  • ListUtils.java: Excluded from the review as it is not my code originally. Simply contains an implementation of shuffle as Collections.shuffle does not work in GWT.



Code

FieldView.java: (39 lines, 760 bytes)

public class FieldView implements IsWidget {

    private static final String HIDDEN_LABEL = "";
    private final Button widget;
    private int value;

    public FieldView(int value) {
        this.value = value;
        widget = new Button(HIDDEN_LABEL);
        widget.setStyleName("game-button", true);
    }

    @Override
    public Button asWidget() {
        return widget;
    }

    public int getValue() {
        return value;
    }

    public void setValue(int value) {
        this.value = value;
    }

    public void showValue() {
        widget.setText(String.valueOf(value));
    }

    public void hideValue() {
        widget.setText(HIDDEN_LABEL);
    }

}


MemoryBoard.java: (86 lines, 2081 bytes)

```
public class MemoryBoard implements IsWidget {

private final Grid grid;
private final Random random = new Random();
private FieldView previousClicked;
private boolean timerRunning;

public MemoryBoard(int width, int height)

Solution

Hmm, very nice! Nice exercise for memory (despite the bit, uhm, sadistic twist),
and the code is nicely written and very hard to pick on: what can be final is final,
arguments are validated, no magic numbers or duplicated string literals, well-formatted and clear, easy to understand. I noticed you pick off the last element from the shuffled list, and even the HTML passes the w3c validator.

As a minor optimization, I would recommend removing the click listeners from the matched tiles that were revealed.

The Random object is used only once, so it could as well be a local variable,
or better yet, just embedded in the statement where you use it.
Later if you want to make this testable, you can consider adding it as a constructor argument. In any case, in the current code there's no visible reason to make it a member field.

Although it's clever to pop off the elements of ints from the end,
how about not removing anything at all:

for (int x = 0; x < width; x++) {
    for (int y = 0; y < height; y++) {
        int value = ints.get(x * width + height);


Although this is more "efficient" in theory,
in your use case this is practically negligible.
You could as well use the short and sweet and inefficient ints.remove(0),
it still wouldn't make a practical difference.

A minor usability note: the numbers on the memory cards are 0-based,
on the example page ranging from 0 to 17.
That's fine for geeks, but regular human beings would probably expect 1-18 instead.

A simple usability improvement that could make the game much more addictive is adding a count of trials. That way I will know if I'm getting better or not, otherwise it's too hard to tell. Further building on this, a history of the scores of recently completed games would be great too.

Code Snippets

for (int x = 0; x < width; x++) {
    for (int y = 0; y < height; y++) {
        int value = ints.get(x * width + height);

Context

StackExchange Code Review Q#67246, answer score: 13

Revisions (0)

No revisions yet.