7
\$\begingroup\$

Introduction

Inspired by this r/dailyprogrammer problem, I've written a Scrabble tile counter. Given a Collection of Scrabble tiles known to be in play, it can output a Map of the remaining unused tiles and their counts, which can be converted into a Map containing counts and a Set of tiles with that count.

Description

Scrabble is a popular word game where players remove tiles with letters on them from a bag and use them to create words on a board. The total number of tiles as well as the frequency of each letter does not change between games.

For this challenge we will be using the tile set from the English edition, which has 100 tiles total. Here's a reference for the distribution and point value of each tile.

Each tile will be represented by the letter that appears on it, with the exception that blank tiles are represented by underscores _.

Input Description

The tiles already in play are inputted as an uppercase string. For example, if 14 tiles have been removed from the bag and are in play, you would be given an input like this:

AEERTYOXMCNB_S

Output Description

You should output the tiles that are left in the bag. The list should be in descending order of the quantity of each tile left in the bag, skipping over amounts that have no tiles.

In cases where more than one letter has the same quantity remaining, output those letters in alphabetical order, with blank tiles at the end.

10: E
9: I
8: A
7: O
5: N, R, T
4: D, L, U
3: G, S
2: F, H, P, V, W
1: B, C, J, K, M, Q, Y, Z, _
0: X

Feedback

  • I'm a pretty new to Java so general pointers around design and implementation would be much appreciated.
  • My biggest design qualm was around the implementation of the ScrabbleTile enum. ScrabbleTiles feel like they should be an enum, with the character and starting count that the tile is associated with.
    • The isScrabbleTile and valueOf methods are fairly inefficient - which makes me wonder if there's a better way to represent ScrabbleTile? Note: I know I haven't implemented anything that uses either but imagine the use case where a method takes in a Collection of chars and returns the tile count. It's something that I might implement in the near future.

Implementation

public static class ScrabbleTileCounter {

  public static Map<ScrabbleTile, Integer> calculateTileCount(final Collection<ScrabbleTile> tiles) throws NoRemainingTilesException {
    final Map<ScrabbleTile, Integer> tileCount = new HashMap<>();
    tileCount.putAll(ScrabbleTile.STARTING_SCRABBLE_TILE_COUNT);
    for (ScrabbleTile tile : tiles) {
      if (tileCount.get(tile) < 1) {
        throw new NoRemainingTilesException();
      }

      final int nextVal = tileCount.get(tile) - 1;
      tileCount.put(tile, nextVal);
    }
    return tileCount;
  }

  public static Map<Integer, Set<ScrabbleTile>> calculateOutputTileCount(final Map<ScrabbleTile, Integer> tileCount) {
    final Map<Integer, Set<ScrabbleTile>> outputTileCount = new HashMap<>();
    for (Map.Entry<ScrabbleTile, Integer> entry : tileCount.entrySet()) {
      final Set<ScrabbleTile> scrabbleTiles = outputTileCount.get(entry.getValue());
      if (scrabbleTiles == null) {
        outputTileCount.put(entry.getValue(), new HashSet<>(Arrays.asList(entry.getKey())));
      } else {
        scrabbleTiles.add(entry.getKey());
        outputTileCount.put(entry.getValue(), scrabbleTiles);
      }
    }
    return outputTileCount;
  }
}

public enum ScrabbleTile {
  A('A', 9),
  B('B', 2),
  C('C', 2),
  D('D', 4),
  E('E', 12),
  F('F', 2),
  G('G', 3),
  H('H', 2),
  I('I', 9),
  J('J', 1),
  K('K', 1),
  L('L', 4),
  M('M', 2),
  N('N', 6),
  O('O', 8),
  P('P', 2),
  Q('Q', 1),
  R('R', 6),
  S('S', 4),
  T('T', 6),
  U('U', 4),
  V('V', 2),
  W('W', 2),
  X('X', 1),
  Y('Y', 2),
  Z('Z', 1),
  BLANK('_', 2),
  ;

  public static final Map<ScrabbleTile, Integer> STARTING_SCRABBLE_TILE_COUNT;
  static {
    final Map<ScrabbleTile, Integer> startingScrabbleTileCount = new HashMap<>();
    for (ScrabbleTile tile : ScrabbleTile.values()) {
      startingScrabbleTileCount.put(tile, tile.getStartingCount());
    }
    STARTING_SCRABBLE_TILE_COUNT = Collections.unmodifiableMap(startingScrabbleTileCount);
  }

  private final char charValue;
  private final int startingCount;

  ScrabbleTile(final char charValue, final int startingCount) {
    this.charValue = charValue;
    this.startingCount = startingCount;
  }

  public char getCharValue() {
    return charValue;
  }

  public int getStartingCount() {
    return startingCount;
  }

  public static boolean isScrabbleTile(final char c) {
    for (final ScrabbleTile tile : ScrabbleTile.values()) {
      if (tile.getCharValue() == c) {
        return true;
      }
    }
    return false;
  }

  public static ScrabbleTile valueOf(final char c) {
    for (final ScrabbleTile tile : ScrabbleTile.values()) {
      if (tile.getCharValue() == c) {
        return tile;
      }
    }

    throw new IllegalArgumentException(String.format("unable to get Scrabble Tile for character: %s", c));
  }
}
\$\endgroup\$
3
\$\begingroup\$

I think that the ScrabbleTile enum is overkill. The tiles are mostly in consecutive ASCIIbetical order (except for the characters [, \, ], and ^ that we would consider as junk between Z and _). A simple array, serving as a lookup table, would do the job:

private static final int[] START_COUNTS = {
    /* A */ 9, /* B */ 2, 2, 4, 12, 2, 3, 2, 9, 1, 1, 4, /* M */ 2,
    /* N */ 6, /* O */ 8, 2, 1, 6, 4, 6, 4, 2, 2, 1, 2, /* Z */ 1,
    0, 0, 0, 0, /* _ */ 2
};

It's much easier just to work with chars and ints directly.


Your calculateTileCount() function appears to implement the bulk of the counting work. I find it to be an unnatural interface design, though. The ScrabbleTileCounter doesn't act like an object, the way its name would suggest. It's not obvious what "calculate" means, or that the tiles is a list of tiles to be removed from the initial set. I would find an interface like this more intuitive:

ScrabbleTileCounter scrabble = new ScrabbleTileCounter();
for (char c : tiles) {
    scrabble.remove(c);
}
/* something here about scrabble.tileCount() */

I don't recommend marking function parameters as final. Functions are usually short enough that it's obvious you haven't reassigned the variable. The final adds noise, and it gives the false impression that the object is immutable, when in fact there is no such guarantee.


I don't think that you need to define a NoRemainingTilesException. The standard NoSuchElementException is close enough to do the job.

\$\endgroup\$
  • \$\begingroup\$ Suggested solution here \$\endgroup\$ – 200_success Jun 23 '16 at 0:06
  • \$\begingroup\$ I also don't like the idea of an enum, but I'm not a big fan of your int[] START_COUNTS either, as if you want to change or check the start counts for 'J' for example, you'd have to count a lot. How about a Map<Character, Integer> ? \$\endgroup\$ – Simon Forsberg Jul 12 '16 at 20:52
  • \$\begingroup\$ @SimonForsberg A Map seems like overkill for such a simple lookup table, but that's a matter of taste. You could also comment the array more generously. \$\endgroup\$ – 200_success Jul 12 '16 at 20:57
  • \$\begingroup\$ If you use have to use comments that excessively to make the code readable, then it's a sign to me that you are initializing them in a bad way. How about reading the start counts from a configuration file and then transform it into an int[] ? \$\endgroup\$ – Simon Forsberg Jul 12 '16 at 21:00
  • \$\begingroup\$ @SimonForsberg That would make a good answer on my follow-up question. \$\endgroup\$ – 200_success Jul 12 '16 at 21:08
2
\$\begingroup\$

EnumMap over HashMap

For enum types, you can use EnumMap over a HashMap as a more efficient Map implementation.

Map.put(K, V) for an existing entry

  final Set<ScrabbleTile> scrabbleTiles = outputTileCount.get(entry.getValue());
  if (scrabbleTiles == null) {
    outputTileCount.put(entry.getValue(), new HashSet<>(Arrays.asList(entry.getKey())));
  } else {
    scrabbleTiles.add(entry.getKey());
    outputTileCount.put(entry.getValue(), scrabbleTiles);
  }

You do not need the second outputTileCount.put(entry.getValue(), scrabbleTiles as scrabbleTiles is a reference to the existing map entry.

Copying Map entries

final Map<ScrabbleTile, Integer> tileCount = new HashMap<>();
tileCount.putAll(ScrabbleTile.STARTING_SCRABBLE_TILE_COUNT);

Both EnumMap (see above) and HashMap lets you copy an existing Map's entries using the constructor too.

Map<ScrabbleTile, Integer> tileCount =
                                new EnumMap(ScrabbleTile.STARTING_SCRABBLE_TILE_COUNT);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.