Memory test in windows forms

Question

I've made a memory test game in windows forms, where you need to find and match 2 cards with the same images.

It has 3 difficulties :

1. Easy - 4 different pairs of cards, no time limit, no mistakes limit.
2. Medium - 8 different pairs of cards, no time limit, 8 mistakes limit.
3. Hard - 16 different pairs of cards, 30 seconds time limit, 16 mistakes limit.

The game starts in the main menu where you can select the difficulty or the play-mode (only single player is available at the moment)

The form is implemented as follows :

public partial class MainMenu : Form
{
    public MainMenu()
    {
        InitializeComponent();
    }

    private void bSinglePlayer_Click(object sender, EventArgs e)
    {
        flpSingleDifficulties.Visible = true;
        flpCoopDifficulties.Visible = false;
    }

    private void bCooperative_Click(object sender, EventArgs e)
    {
        flpCoopDifficulties.Visible = true;
        flpSingleDifficulties.Visible = false;
    }

    private void bExit_Click(object sender, EventArgs e)
    {
        Application.Exit();
    }

    private void OnGameClosing(object sender, FormClosingEventArgs e)
    {
        if (InvokeRequired)
        {
            this.Invoke(new MethodInvoker(Show));
        }
    }

    private void bEasySingle_Click(object sender, EventArgs e)
    {
        SinglePlayerEasy singleEasyForm = new SinglePlayerEasy();
        Task.Run(() => singleEasyForm.ShowDialog());
        singleEasyForm.FormClosing += OnGameClosing;
        Hide();
    }

    private void bMediumSingle_Click(object sender, EventArgs e)
    {
        SinglePlayerMedium singleMediumForm = new SinglePlayerMedium();
        Task.Run(() => singleMediumForm.ShowDialog());
        singleMediumForm.FormClosing += OnGameClosing;
        Hide();
    }

    private void bHardSingle_Click(object sender, EventArgs e)
    {
        SinglePlayerHard singleHardForm = new SinglePlayerHard();
        Task.Run(() => singleHardForm.ShowDialog());
        singleHardForm.FormClosing += OnGameClosing;
        Hide();
    }

    private void bEasyCoop_Click(object sender, EventArgs e)
    {

    }

    private void bMediumCoop_Click(object sender, EventArgs e)
    {

    }

    private void bHardCoop_Click(object sender, EventArgs e)
    {

    }
}

The 3 buttons next to the Single Player button are shown only after you press the game mode and there are 2 separate panels for each difficulty consisting of the exact same looking buttons but with different event handlers.

In order to avoid the annoyance of having multiple forms opened when you are just using a single one, I'm hiding the Main Menu form once you start the game, and I'm showing it once again when the game form is closed that's why I need to subscribe to the newly created form' FormClosing event.

I also have a Settings class which contains stuff like all the card Images for example :

public static class Settings
{
    public enum CardCountByDifficulty
    {
        Easy = 8,
        Medium = 16,
        Hard = 32,
    }

    public static Image[] CardImages { get; }
    public static Image BackImage { get; }

    public const int Offset = 2;

    static Settings()
    {
        CardImages = Directory.GetFiles(@"Assets\", "*.jpg", SearchOption.TopDirectoryOnly).Select(Image.FromFile).ToArray();
        BackImage = Image.FromFile(@"Assets\Back.jpg");
    }
}

I've separated all the 3 difficulties in 3 forms instead of having them in 1. All the cards in the game are using 1 simple class - Card :

public class Card
{
    public int ImageIndex { get; }
    private readonly PictureBox Holder;

    public Card(PictureBox holder, int imageIndex)
    {
        ImageIndex = imageIndex;
        Holder = holder;
    }

    public bool IsFlipped => Holder.Image == null;

    public void Hide() => Holder.Image = Settings.BackImage;
    public void Flip() => Holder.Image = null;
}

The way the cards function is that whenever I want to show a card I set the Holder.Image to null (which if not set to null has the value of Settings.BackImage) revealing the BackgroundImage of the picturebox which is the actual image of the card.

You might be wondering why do I need the ImageIndex property well that's because it's a lot easier to compare if 2 cards are equal by using those indexes instead of comparing images. We are taking this integer value from the card's image index in the Settings.CardImages[] (I will show this in a moment). This also makes shuffling cards a lot easier.

Each separate form has it's own array of CardImage which has only 2 properties one is the actual Image and other is used to retain the original index from the Settings.CardImages array.

public class CardImage
{
    public int ActualIndex { get; }
    public Image Image { get; }

    public CardImage(Image image, int actualIndex)
    {
        Image = image;
        ActualIndex = actualIndex;
    }
}

I also have a shared file for all the common logic which is a singleton contain mostly public methods :

public sealed class Common
{
    public delegate void CardClick(Card sender);

    private static Common instance = null;
    private static readonly object padlock = new object();

    public static Common Instance
    {
        get
        {
            lock (padlock)
            {
                return instance ?? (instance = new Common());
            }
        }
    }

    private static readonly Random random = new Random();

    private Common()
    {

    }

    public CardImage[] GetImages(Settings.CardCountByDifficulty cardCountByDifficulty)
    {
        CardImage[] images = new CardImage[(int) cardCountByDifficulty];
        int index = 0;
        for (int i = 0; i < images.Length; i++)
        {
            images[i] = new CardImage(Settings.CardImages[index], index);
            if (i%2 == 1 && i > 0)
            {
                index++;
            }
        }
        return images;
    }

    public List<Card> CreateCards(Control parent, CardImage[] images, int rowCount, int columnCount,
        CardClick onCardClick)
    {
        PictureBox[] pictureBoxs = new PictureBox[images.Length];
        List<Card> cards = new List<Card>();

        Func<int, int, int> calculateSize =
            (axisSize, axisCount) => (axisSize - Settings.Offset*(axisCount + axisCount - 1))/axisCount;
        Size cardSize = new Size(calculateSize(parent.Width, columnCount), calculateSize(parent.Height, rowCount));

        for (int i = 0; i < pictureBoxs.Length; i++)
        {
            pictureBoxs[i] = CreateCardPictureBox(images[i].Image, cardSize);
            parent.Controls.Add(pictureBoxs[i]);

            cards.Add(new Card(pictureBoxs[i], images[i].ActualIndex));
            Card currentCard = cards.Last(); // closure avoidance
            pictureBoxs[i].Click += (sender, args) => onCardClick(currentCard);
        }
        return cards;
    }

    private static PictureBox CreateCardPictureBox(Image image, Size cardSize)
    {
        return new PictureBox
        {
            Size = cardSize,
            BackgroundImageLayout = ImageLayout.Stretch,
            SizeMode = PictureBoxSizeMode.StretchImage,
            BackgroundImage = image,
            Image = Settings.BackImage,
            Margin = new Padding(Settings.Offset)
        };
    }

    public IList<T> Shuffle<T>(IList<T> array)
    {
        int n = array.Count;
        for (int i = 0; i < n; i++)
        {
            int r = i + (int) (random.NextDouble()*(n - i));
            T t = array[r];
            array[r] = array[i];
            array[i] = t;
        }
        return array;
    }

    public bool HasWon(IList<Card> cards)
    {
        return cards.Count == 0;
    }

    public void Win(Action action)
    {
        ShowEndGameMessageBox($@"You won ! {Environment.NewLine}Would you like to play again ?", @"Congratulations", action);
    }

    public void Loose(Action action)
    {
        ShowEndGameMessageBox($@"You lost ! {Environment.NewLine}Would you like to play again ?", "Game over", action);
    }

    public void DisposeCards(Control parent)
    {
        while (parent.Controls.Count > 0)
        {
            parent.Controls[0].Dispose();
        }
    }

    public async Task ShowCardsOnStartup(IList<Card> cards, int timeToShowInMilliseconds)
    {
        foreach (var card in cards)
        {
            card.Flip();
        }
        await Task.Delay(timeToShowInMilliseconds);
        foreach (var card in cards)
        {
            card.Hide();
        }
    }

    private static void ShowEndGameMessageBox(string text, string title, Action action)
    {
        DialogResult messageBoxResult = MessageBox.Show(text, title, MessageBoxButtons.YesNo);
        switch (messageBoxResult)
        {
            case DialogResult.Yes:
                action();
                break;
            case DialogResult.No:
                DialogResult exitMessageBoxResult =
                    MessageBox.Show(@"Are you sure you want to quit the application ?", @"Exit",
                        MessageBoxButtons.YesNo);
                if (exitMessageBoxResult == DialogResult.Yes)
                {
                    Application.Exit();
                }
                else
                {
                    action();
                }
                break;
        }
    }
}

I wont go over the methods here since they're quite short and mostly self-explanatory (but if something is unclear I'm happy to answer in the comments).

A quick summary of how the game actually works (this is valid for all the 3 forms with small modifications) :

OnCardClick

If the pressed card is flipped we have nothing to do here. Else we flip the card and check if we have a previously selected card. If we have one we compare > the indexes if they are equal we keep them shown, else we hide the 2 cards.

Restart

Dispose all the cards, shuffle the images and create new ones.

Let's start with the easy game mode :

Please ignore the fact that the cards look ridiculous, I like them that way.

public sealed partial class SinglePlayerEasy : Form
{
    private List<Card> cards;
    private readonly CardImage[] cardsImages;

    private Card previousClickedCard;

    private bool isShowingCards = false;

    public SinglePlayerEasy()
    {
        InitializeComponent();
        cardsImages = Common.Instance.GetImages(Settings.CardCountByDifficulty.Easy);
        Common.Instance.Shuffle(cardsImages);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 4, OnCardClick);
    }

    protected override CreateParams CreateParams
    {
        get
        {
            CreateParams cp = base.CreateParams;
            cp.ExStyle |= 0x02000000;
            return cp;
        }
    }

    private async void OnCardClick(Card sender)
    {
        if (sender.IsFlipped || isShowingCards)
        {
            return;
        }
        sender.Flip();
        if (previousClickedCard == null)
        {
            previousClickedCard = sender;
        }
        else
        {
            if (previousClickedCard.ImageIndex != sender.ImageIndex)
            {
                isShowingCards = true;
                await Task.Delay(250);
                isShowingCards = false;
                previousClickedCard.Hide();
                sender.Hide();
            }
            else
            {
                cards.Remove(previousClickedCard);
                cards.Remove(sender);
                if (Common.Instance.HasWon(cards))
                {
                    Common.Instance.Win(Restart);
                    return;
                }
            }
            previousClickedCard = null;
        }
    }

    private void Restart()
    {
        previousClickedCard = null;
        Common.Instance.Shuffle(cardsImages);
        Common.Instance.DisposeCards(flpCardHolder);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 4, OnCardClick);
    }
}

The medium form has as mentioned earlier mistakes count so that's something new added into the click event handler + we show all the cards for 1 second before we hide them again.

And this is the code for it, it's pretty similar to the first one :

public partial class SinglePlayerMedium : Form
{
    private const int timeToShowCards = 1000;

    private List<Card> cards;
    private readonly CardImage[] cardsImages;

    private Card previousClickedCard;

    private bool isShowingCards = false;

    private int _mistakesCount;
    private int mistakesCount
    {
        get { return _mistakesCount; }
        set
        {
            _mistakesCount = value;
            lbMistakes.Text = $@"Mistakes {mistakesCount} / {maxMistakes}";
        }
    }

    private const int maxMistakes = 8;

    public SinglePlayerMedium()
    {
        InitializeComponent();
        lbMistakes.Text = $@"Mistakes {mistakesCount} / {maxMistakes}";
        cardsImages = Common.Instance.GetImages(Settings.CardCountByDifficulty.Medium);
        Common.Instance.Shuffle(cardsImages);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 8, OnCardClick);
    }

    protected override CreateParams CreateParams
    {
        get
        {
            CreateParams cp = base.CreateParams;
            cp.ExStyle |= 0x02000000;
            return cp;
        }
    }

    private async void OnCardClick(Card sender)
    {
        if (sender.IsFlipped || isShowingCards)
        {
            return;
        }
        sender.Flip();
        if (previousClickedCard == null)
        {
            previousClickedCard = sender;
        }
        else
        {
            if (previousClickedCard.ImageIndex != sender.ImageIndex)
            {
                mistakesCount++;
                if (mistakesCount >= maxMistakes)
                {
                    Common.Instance.Loose(Restart);
                    return;
                }
                isShowingCards = true;
                await Task.Delay(250);
                isShowingCards = false;
                previousClickedCard.Hide();
                sender.Hide();
            }
            else
            {
                cards.Remove(previousClickedCard);
                cards.Remove(sender);
                if (Common.Instance.HasWon(cards))
                {
                    Common.Instance.Win(Restart);
                    return;
                }
            }
            previousClickedCard = null;
        }
    }

    private async void Restart()
    {
        previousClickedCard = null;
        mistakesCount = 0;
        Common.Instance.Shuffle(cardsImages);
        Common.Instance.DisposeCards(flpCardHolder);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 8, OnCardClick);
        await Common.Instance.ShowCardsOnStartup(cards, timeToShowCards);
    }

    private async void SinglePlayerMedium_Shown(object sender, System.EventArgs e)
    {
        await Common.Instance.ShowCardsOnStartup(cards, timeToShowCards);
    }
}

I'm using the Shown event because creating the buttons seems to take most of the little to none power that windows forms has and if we show the cards in the constructor they wont be shown for 1 full second but instead maybe 400-500 milliseconds because the first time the form is built, all the controls go black for a few moments.

The hard mode has 2 new additions regarding the medium difficulty - time limit of 30 seconds + a Joker button which works the following way :

If we have a selected card look into the array of Cards and find the one that has the same image index and flip it.

Else pick a random image index and flip the 2 cards that have the same index.

public partial class SinglePlayerHard : Form
{     
    private readonly Timer timer;
    private const int startingTime = 30;

    private const int timeToShowCards = 2000;

    private List<Card> cards;
    private readonly CardImage[] cardsImages;

    private Card previousClickedCard;

    private bool isShowingCards = false;
    
    private int _mistakesCount;
    private int mistakesCount
    {
        get { return _mistakesCount; }
        set
        {
            _mistakesCount = value;
            lbMistakes.Text = $@"Mistakes {mistakesCount} / {maxMistakes}";
        }
    }

    private const int maxMistakes = 16;

    private readonly Random random = new Random();

    public SinglePlayerHard()
    {
        InitializeComponent();
        lbMistakes.Text = $@"Mistakes {mistakesCount} / {maxMistakes}";

        cardsImages = Common.Instance.GetImages(Settings.CardCountByDifficulty.Hard);
        Common.Instance.Shuffle(cardsImages);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 4, 8, OnCardClick);

        timer = new Timer { Interval = 1000 };
        timer.Tick += Timer_Tick;
        pbTimer.Maximum = startingTime;
        pbTimer.Value = pbTimer.Maximum;
    }

    private void Timer_Tick(object sender, EventArgs e)
    {
        pbTimer.Value--;
        if (pbTimer.Value <= 0)
        {
            timer.Stop();
            Common.Instance.Loose(Restart);
        }
    }

    protected override CreateParams CreateParams
    {
        get
        {
            CreateParams cp = base.CreateParams;
            cp.ExStyle |= 0x02000000;
            return cp;
        }
    }

    private void bJoker_Click(object sender, EventArgs e)
    {
        if (previousClickedCard == null)
        {
            int imageIndex = cards[random.Next(0, cards.Count)].ImageIndex;
            foreach (var card in cards.Where(c => c.ImageIndex == imageIndex).ToArray())
            {
                card.Flip();
                cards.Remove(card);
            }
        }
        else
        {
            Card card = cards.First(c => c.ImageIndex == previousClickedCard.ImageIndex && c != previousClickedCard);
            card.Flip();
            cards.Remove(card);
            cards.Remove(previousClickedCard);
        }
        previousClickedCard = null;
        if (Common.Instance.HasWon(cards))
        {
            timer.Stop();
            Common.Instance.Win(Restart);
        }
    }

    private async void OnCardClick(Card sender)
    {
        if (sender.IsFlipped || isShowingCards)
        {
            return;
        }
        sender.Flip();
        if (previousClickedCard == null)
        {
            previousClickedCard = sender;
        }
        else
        {
            if (previousClickedCard.ImageIndex != sender.ImageIndex)
            {
                mistakesCount++;
                if (mistakesCount >= maxMistakes)
                {
                    timer.Stop();
                    Common.Instance.Loose(Restart);
                    return;
                }
                isShowingCards = true;
                await Task.Delay(250);
                isShowingCards = false;
                previousClickedCard.Hide();
                sender.Hide();
            }
            else
            {
                cards.Remove(previousClickedCard);
                cards.Remove(sender);
                if (Common.Instance.HasWon(cards))
                {
                    timer.Stop();
                    Common.Instance.Win(Restart);
                    return;
                }
            }
            previousClickedCard = null;
        }
    }

    private async void Restart()
    {
        pbTimer.Value = pbTimer.Maximum;
        previousClickedCard = null;
        mistakesCount = 0;
        Common.Instance.Shuffle(cardsImages);
        Common.Instance.DisposeCards(flpCardHolder);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 4, 8, OnCardClick);
        await Common.Instance.ShowCardsOnStartup(cards, timeToShowCards);
        timer.Start();
    }

    private async void SinglePlayerHard_Shown(object sender, EventArgs e)
    {
        await Common.Instance.ShowCardsOnStartup(cards, timeToShowCards);
        timer.Start();
    }
}

That's it !

I was planning to make a cooperative mode that's why all of the forms have the prefix SinglePlayer, however windows forms doesn't seems to handle well the current state of the program and my idea for the cooperative mode is going to be a little bit more heavy so I scrapped that for now, unless the nice people at CodeReview have good tips regarding the performance of the program, I'm also looking for any code style, code shortening tips.

Thank you for taking your time to read this.

Answer 1

I guess you already know that having three almost identical forms isn't very good. In case there is a bug somewhere, you need to fix it three times.

Let me try to show you how you could implement it with only one form. Keep in mind that I couldn't actually test it and I wrote it in notepad. It might have some bugs or might not (or rather certainly isn't) be complete or even compile but it should give you a rough idea how to start.

---

The SinglePlayerEasy form seems to have everything you need to play the game. The other forms just extend it. I suggest renaming it to SinglePlayerForm and move the logic of other levels to separate classes.

I name it SinglePlayerLevel and I start with two methods: one that extends the form and an event handler for the card-click. By the way, your CardClick delegate does not follow the event handler convention although it tries to be one.

---

I picked an abstract class because it provides the GameOver event. You may need to handle it in the hard-level where the timer runs out.

abstract class SinglePlayerLevel
{
    public event EventHandler GameOver { get; set; }
    public abstract void Initialize(SinglePlayerForm form);
    protected abstract void CardClick(object sender, CardClickEventArgs e);
}

The derived classes implement the logic for each level.

In the easy one there is probably nothing to do...

class SinglePlayerEasy : SinglePlayerLevel
{
    public override void Initialize(SinglePlayerForm form) {..}
    protected override void CardClick(object sender, CardClickEventArgs e) {..}
}

---

The medium level adds some new stuff like new controls and card-click logic. Use the Initialize method to extend the form and the event handler to implement the custom card-click handling.

class SinglePlayerMedium : SinglePlayerLevel
{
    public SinglePlayerMedium()
    {
    }

    private int _mistakesCount;
    private int mistakesCount
    {
        get { return _mistakesCount; }
        set
        {
            _mistakesCount = value;
            lbMistakes.Text = $@"Mistakes {mistakesCount} / {maxMistakes}";
        }
    }

    private const int maxMistakes = 8;

    public override void Initialize(SinglePlayerForm form)
    {
        // add mistakesLabel
        // add event handlers
        form.CardClick += CardClick;
    }

    protected override void CardClick(object sender, CardClickEventArgs e)
    {
        mistakesCount++;
        if (mistakesCount >= maxMistakes)
        {
            Common.Instance.Loose(Restart);
            e.Handled = true;
        }
    }
}

---

Do the same for the hard-level. Add timers, labels, buttons, events, etc.

class SinglePlayerHard : SinglePlayerLevel
{
    // implement it for the hard-logic and extensions
}

---

The last part to change is the SinglePlayerForm. It needs two constructors. The default one that the designer needs and another one that requries a level and that you can use at runtime to create the form for the chosen level.

In this new form the OnCardClick (this needs to be fixed) method raises the CardClick event so that the SinglePlayerLevel implementation can handle it.

It is passed a new type of EventArgs:

class CardClickEventArgs : EventArgs
{
    public CardClickEventArgs() {..}
    public Card Card { get; } // set in the constructor
    public bool Handled { get; set; }
}

---

This is used with the new CardClick event that I've added to this form.

public partial class SinglePlayerForm : Form
{
    private readonly SinglePlayerLevel _level;
    private List<Card> cards;
    private readonly CardImage[] cardsImages;

    private Card previousClickedCard;

    private bool isShowingCards = false;

    public SinglePlayerForm()
    {
        InitializeComponent();
        cardsImages = Common.Instance.GetImages(Settings.CardCountByDifficulty.Easy);
        Common.Instance.Shuffle(cardsImages);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 4, OnCardClick);
    }

    public SinglePlayerForm(SinglePlayerLevel level) : this()
    {
        _level = level;
        _level.Initialize(this);
    }

    public event EventHandler<CardClickEventArgs> CardClick { get; set; }

    protected override CreateParams CreateParams
    {
        get
        {
            CreateParams cp = base.CreateParams;
            cp.ExStyle |= 0x02000000;
            return cp;
        }
    }

    private async void OnCardClick(Card sender)
    {
        if (sender.IsFlipped || isShowingCards)
        {
            return;
        }
        sender.Flip();
        if (previousClickedCard == null)
        {
            previousClickedCard = sender;
        }
        else
        {
            if (previousClickedCard.ImageIndex != sender.ImageIndex)
            {
                var e = new CardClickEventArgs(sender); 
                CardClick(this, e);
                if (e.Handled) { return; }

                isShowingCards = true;
                await Task.Delay(250);
                isShowingCards = false;
                previousClickedCard.Hide();
                sender.Hide();
            }
            else
            {
                cards.Remove(previousClickedCard);
                cards.Remove(sender);
                if (Common.Instance.HasWon(cards))
                {
                    Common.Instance.Win(Restart);
                    return;
                }
            }
            previousClickedCard = null;
        }
    }

    private void Restart()
    {
        previousClickedCard = null;
        Common.Instance.Shuffle(cardsImages);
        Common.Instance.DisposeCards(flpCardHolder);
        cards = Common.Instance.CreateCards(flpCardHolder, cardsImages, 2, 4, OnCardClick);
    }
}

---

This design will allow to add more levels anytime and to customize the basic-form. I also think with small adjustments (if necessary at all) you should be able to reuse the level definitions for multiplayer.

---

You should know that the Common class also has issues but I currently haven't any ideas how to improve it... in fact I didn't really tried hard enough ;-)

Answer 2

private void bEasySingle_Click(object sender, EventArgs e)
{
    SinglePlayerEasy singleEasyForm = new SinglePlayerEasy();
    Task.Run(() => singleEasyForm.ShowDialog());
    singleEasyForm.FormClosing += OnGameClosing;
    Hide();
}

Looks like you're running the new window in a new thread simply to hide the current one until the new one is closed. If so, why not just call Hide before showing the new window, then call Show when the new window is closed?

private void bEasySingle_Click(object sender, EventArgs e)
{
    SinglePlayerEasy singleEasyForm = new SinglePlayerEasy();
    Hide();
    singleEasyForm.ShowDialog();
    Show();
}

---

private async void OnCardClick(Card sender)
{
    if (sender.IsFlipped || isShowingCards)
    {
        return;
    }
    sender.Flip();

sender.Flip(), isn't it clearer to call the parameter card so you can say card.Flip()?

---

Also (though I don't have time to look at much less write it in detail) it seems to me your easy/medium/hard classes should all be inheriting from a base class that implements the common methods and functionality, so you don't have to have so much duplicate code in each class.