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

Populating a bag of Scrabble tiles

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

Problem

I am filling a bag full of scrabble tiles. There should be 100 tiles in the bag of these letters:

A-9, B-2, C-2, D-4, E-12, F-2, G-3, H-2, I-9, J-1, K-1, L-4, M-2, N-6, O-8, P-2, Q-1, R-6, S-4, T-6, U-4, V-2, W-2, X-1, Y-2, Z-1 and Blanks-2.


These numbers/letters should never change. I have a working solution, but it is horrible. There is a lot of duplicate code, a lot of room for errors, and it has very poor readability. I am stumped on how I can make this code more readable and less error-prone. Any suggestions? I am not opposed to scratching this class altogether, if anyone knows of a better approach.

Bag.cs

```
public class Bag
{
private Tile[] tiles = new Tile[100];
private const int numA = 9;
private const int numB = 2;
private const int numC = 2;
private const int numD = 4;
private const int numE = 12;
private const int numF = 2;
private const int numG = 3;
private const int numH = 2;
private const int numI = 9;
private const int numJ = 1;
private const int numK = 1;
private const int numL = 4;
private const int numM = 2;
private const int numN = 6;
private const int numO = 8;
private const int numP = 2;
private const int numQ = 1;
private const int numR = 6;
private const int numS = 4;
private const int numT = 6;
private const int numU = 4;
private const int numV = 2;
private const int numW = 2;
private const int numX = 1;
private const int numY = 2;
private const int numZ = 1;
private const int numBlank = 2;

public void populateTiles()
{
int i = 0;
for (int j = 0; j < numA; j++)
{
tiles[i] = new Tile('A');
i++;
}
for (int j = 0; j < numB; j++)
{
tiles[i] = new Tile('B');
i++;
}
for (int j = 0; j < numC; j++)
{
tiles[i] = new Tile('C');
i++;
}
for (int j = 0; j < n

Solution

Forewarning: I'm not the most experienced C# programmer, so if anything is wrong, just mention it.

There is way too much useless repetition here. This begs for an implementation using System.Collections.Generic.Dictionary. You can do something like this to store the possible tiles, and their counts.

public Dictionary possibleTiles = new Dictionary() {
    {'a', 9},
    {'b', 2},
    {'c', 2},
    ...
};


You can then loop over this dictionary like this. You would use the key as the tile to be added to your tiles array, and the value as the amount of those specific tiles.

foreach(KeyValuePair item in possibleTiles)
{
    // Add item.key item.value times to the tiles array.
}


Finally, for reference, here's the my refactored version of the Bag class. Do note though, it uses List rather than an array.

using System;
using System.Collections.Generic;

/// 
/// Represents a bag of scrabble tiles.
/// 
class Bag
{
    public List tiles = new List();
    private Dictionary possibleTiles = new Dictionary() {
        {'a', 9}, {'b', 2}, {'c', 2}, {'d', 4}, {'e', 12},
        {'f', 2}, {'g', 3}, {'h', 2}, {'i', 9}, {'j', 1},
        {'k', 1}, {'l', 4}, {'m', 2}, {'n', 6}, {'o', 8},
        {'p', 2}, {'q', 1}, {'r', 6}, {'s', 4}, {'t', 6},
        {'u', 4}, {'v', 2}, {'w', 2}, {'x', 1}, {'y', 2},
        {'z', 1}, {' ', 2}
    };

    /// 
    /// Populate the list tiles with scrabble tiles.
    /// 
    public void PopulateTiles()
    {
        foreach(KeyValuePair item in possibleTiles)
        {
            Tile tile = new Tile(item.Key);
            int amount = item.Value; 
            for(int n = 0; n <= amount; n++)
            {
                tiles.Add(tile);
            }
        }
    }
}

Code Snippets

public Dictionary<char, int> possibleTiles = new Dictionary<char, int>() {
    {'a', 9},
    {'b', 2},
    {'c', 2},
    ...
};
foreach(KeyValuePair<char, int> item in possibleTiles)
{
    // Add item.key item.value times to the tiles array.
}
using System;
using System.Collections.Generic;

/// <summary>
/// Represents a bag of scrabble tiles.
/// </summary>
class Bag
{
    public List<Tile> tiles = new List<Tile>();
    private Dictionary<char, int> possibleTiles = new Dictionary<char, int>() {
        {'a', 9}, {'b', 2}, {'c', 2}, {'d', 4}, {'e', 12},
        {'f', 2}, {'g', 3}, {'h', 2}, {'i', 9}, {'j', 1},
        {'k', 1}, {'l', 4}, {'m', 2}, {'n', 6}, {'o', 8},
        {'p', 2}, {'q', 1}, {'r', 6}, {'s', 4}, {'t', 6},
        {'u', 4}, {'v', 2}, {'w', 2}, {'x', 1}, {'y', 2},
        {'z', 1}, {' ', 2}
    };

    /// <summary>
    /// Populate the list tiles with scrabble tiles.
    /// </summary>
    public void PopulateTiles()
    {
        foreach(KeyValuePair<char, int> item in possibleTiles)
        {
            Tile tile = new Tile(item.Key);
            int amount = item.Value; 
            for(int n = 0; n <= amount; n++)
            {
                tiles.Add(tile);
            }
        }
    }
}

Context

StackExchange Code Review Q#94202, answer score: 13

Revisions (0)

No revisions yet.