Search code examples
c#user-interfacereferencepass-by-referencedice

Parameters passed by reference Rolling Dice GUI C#


I've got this working, but I'm trying to find an easier way to do this.

I need a program that displays a picture of two dice, and I am required to have a Class, and at least one method that correctly uses parameters that are passed by reference.

I've got my GetRoll method in my class using two parameters passed by reference, but the only way I've been able to get this working is by making a ridiculous amount of if else statements. There's got to be a better way. Any ideas? Here's my 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;

namespace DiceGame
{
    public partial class Form1 : Form
    {
        DiceClass objectRef;
        public Form1()
        {
            InitializeComponent();
            objectRef = new DiceClass();
        }    
        private void rollEm_Click(object sender, EventArgs e)
        {
            specialMessage.Text = "";
            objectRef.RollEm();
            string str1 = "";
            string str2 = "";
            objectRef.GetRoll(ref str1, ref str2);
            die1.Text = str1;
            die2.Text = str2;
            if (objectRef.BoxCars())
            {
                specialMessage.Text = "BOX CARS!!";
            }
            else
            {
                if (!objectRef.SnakeEyes())
                    return;
                specialMessage.Text = "SNAKE EYES!!";
            }
        }
    }
}

And here's my class:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace DiceGame
{
    class DiceClass
    {
        private static string nL = Environment.NewLine;
        string one = nL + " l ";
        string two = "l" + nL + nL + "  l";
        string three = "l l" + nL + nL + "l l";
        string four = "l l" + nL + nL + "l l";
        string five = "l l" + nL + " l " + nL + "l l";
        string six = "l l" + nL + "l l" + nL + "l l";
        private const int BOX = 6;
        private int firstDie;
        private int secondDie;
        Random randomNums = new Random();
        public DiceClass()
        {
            firstDie = 0;
            secondDie = 0;
        }

        public void RollEm()
        {
            firstDie = randomNums.Next(1, 7);
            secondDie = randomNums.Next(1, 7);
        }
        public bool BoxCars()
        {
            return firstDie == 6 && secondDie == 6;
        }

        public bool SnakeEyes()
        {
            return firstDie == 1 && secondDie == 1;
        }
        // is there an easier way to have this method work without all these if else statements??
        public void GetRoll(ref string first, ref string second)
        {
            if (firstDie == 1 && secondDie == 1)
            {
                first = one;
                second = one;
            }
            else if (firstDie == 1 && secondDie == 2)
            {
                first = one;
                second = two;
            }
            else if (firstDie == 1 && secondDie == 3)
            {
                first = one;
                second = three;
            }
            else if (firstDie == 1 && secondDie == 4)
            {
                first = one;
                second = four;
            }
            else if (firstDie == 1 && secondDie == 5)
            {
                first = one;
                second = five;
            }
            else if (firstDie == 1 && secondDie == 6)
            {
                first = one;
                second = six;
            }
            else if (firstDie == 2 && secondDie == 1)
            {
                first = two;
                second = one;
            }
            else if (firstDie == 2 && secondDie == 2)
            {
                first = two;
                second = two;
            }
            else if (firstDie == 2 && secondDie == 3)
            {
                first = two;
                second = three;
            }
            else if (firstDie == 2 && secondDie == 4)
            {
                first = two;
                second = four;
            }
            else if (firstDie == 2 && secondDie == 5)
            {
                first = two;
                second = five;
            }
            else if (firstDie == 2 && secondDie == 6)
            {
                first = two;
                second = six;
            }
            else if (firstDie == 3 && secondDie == 1)
            {
                first = three;
                second = one;
            }
            else if (firstDie == 3 && secondDie == 2)
            {
                first = three;
                second = two;
            }
            else if (firstDie == 3 && secondDie == 3)
            {
                first = three;
                second = three;
            }
            else if (firstDie == 3 && secondDie == 4)
            {
                first = three;
                second = four;
            }
            else if (firstDie == 3 && secondDie == 5)
            {
                first = three;
                second = five;
            }
            else if (firstDie == 3 && secondDie == 6)
            {
                first = three;
                second = six;
            }
            else if (firstDie == 4 && secondDie == 1)
            {
                first = four;
                second = one;
            }
            else if (firstDie == 4 && secondDie == 2)
            {
                first = four;
                second = two;
            }
            else if (firstDie == 4 && secondDie == 3)
            {
                first = four;
                second = three;
            }
            else if (firstDie == 4 && secondDie == 4)
            {
                first = four;
                second = four;
            }
            else if (firstDie == 4 && secondDie == 5)
            {
                first = four;
                second = five;
            }
            else if (firstDie == 4 && secondDie == 6)
            {
                first = four;
                second = six;
            }
            else if (firstDie == 5 && secondDie == 1)
            {
                first = five;
                second = one;
            }
            else if (firstDie == 5 && secondDie == 2)
            {
                first = five;
                second = two;
            }
            else if (firstDie == 5 && secondDie == 3)
            {
                first = five;
                second = three;
            }
            else if (firstDie == 5 && secondDie == 4)
            {
                first = five;
                second = four;
            }
            else if (firstDie == 5 && secondDie == 5)
            {
                first = five;
                second = five;
            }
            else if (firstDie == 5 && secondDie == 6)
            {
                first = five;
                second = six;
            }
            else if (firstDie == 6 && secondDie == 1)
            {
                first = six;
                second = one;
            }
            else if (firstDie == 6 && secondDie == 2)
            {
                first = six;
                second = two;
            }
            else if (firstDie == 6 && secondDie == 3)
            {
                first = six;
                second = three;
            }
            else if (firstDie == 6 && secondDie == 4)
            {
                first = six;
                second = four;
            }
            else if (firstDie == 6 && secondDie == 5)
            {
                first = six;
                second = five;
            }
            else
            {
                first = six;
                second = six;
            }
        }
    }
}

Solution

  • Use a dictionary do define your mapping once:

    Dictionary<int, string> dieRapping = new Dictionary<int, string>() {
        { 1, nL + " l " },
        { 2, "l" + nL + nL + "  l" },
        { 3, "l l" + nL + nL + "l l" },
        { 4, "l l" + nL + nL + "l l" },
        { 5, "l l" + nL + " l " + nL + "l l" },
        { 6, "l l" + nL + "l l" + nL + "l l" }
    };
    

    Now you can convert integers to strings in constant time by just using dieRapping[someInteger]. So your whole GetRoll method becomes this:

    public void GetRoll(ref string first, ref string second)
    {
        first = dieRapping[firstDie];
        second = dieRapping[secondDie];
    }
    

    Note that even without a mapping, you could have saved yourself a lot if you treated first and second independently. Since they do not depend on another, you can just handle first first, and then do second:

    if (firstDie == 1)
        first = one;
    else if (firstDie == 2)
        first = two;
    else …
    
    if (secondDie == 1)
        second = one;
    else if (secondDie == 2)
        second = two;
    else …
    

    Or, since the logic is the same for first and second, you could have introduced another method:

    public string GetSingleRoll(int value)
    {
        if (value == 1)
            return one;
        else if (value == 2)
            return two;
        else …
    }
    

    And then you could just call that method twice in GetRoll:

    first = GetSingleRoll(firstDie);
    second = GetSingleRoll(secondDie);
    

    But that are just some ways to reduce repetition. In your case, using a mapping is probably the best solution.