Creating minesweeper levels to C# console

Question

I recently did a technical test for a potential coding job. In short, the test asked that I write code that would generate random minesweeper levels to console of specified grid dimensions and mine amounts.

This is the code I submitted:

using System;
namespace MineSweeper_Level_Generator
{
 /// <summary>
 /// Minefield class to generate Minesweeper grids of assigned sizes.
 /// </summary>
 class Minefield
 {
 /// <summary>
 /// Enum to define the states of each cell in the grid.
 /// </summary>
 public enum Cell {
 EMPTY,
 M1,
 M2,
 M3,
 M4, 
 M5,
 M6,
 M7,
 M8, 
 M9,
 MINE
 };
 public static int gridWidth, gridHeight, mineCount;
 /// <summary>
 /// The entry point of the program, where the program control starts and ends.
 /// This function will gather input values from the user, 
 /// generate a minefield and then print the grid to console.
 /// </summary>
 /// <param name="args">The command-line arguments.</param>
 public static void Main (string[] args) {
 Cell[,] gridCells;
 GetInput ();
 gridCells = GenerateMineField (gridWidth, gridHeight, mineCount);
 PrintField (gridCells);
 }
 /// <summary>
 /// Gets console input from the user for the width and height of the minefield,
 /// as well as how many mines are to be place in the field.
 /// </summary>
 static void GetInput() {
 string input = "";
 Console.WriteLine ("Enter width:");
 input = Console.ReadLine ();
 // Forces user to continue inputting values until a valid number is entered.
 while (!Int32.TryParse (input, out gridWidth)) {
 Console.WriteLine ("This is not a valid number. Please enter a valid width:");
 input = Console.ReadLine ();
 }
 Console.WriteLine ("Enter height:");
 input = Console.ReadLine ();
 // Forces user to continue inputting values until a valid number is entered.
 while (!Int32.TryParse (input, out gridHeight)) {
 Console.WriteLine ("This is not a valid number. Please enter a valid height:");
 input = Console.ReadLine ();
 }
 Console.WriteLine ("Enter mine count:");
 input = Console.ReadLine ();
 // Forces user to continue inputting values until a valid number is entered.
 // The number also needs to be within the amount of cells in the grid to be valid.
 while (!Int32.TryParse (input, out mineCount) || Convert.ToInt32 (input) > gridWidth * gridHeight) {
 if (!Int32.TryParse (input, out mineCount)) {
 Console.WriteLine ("This is not a valid number. Please enter a valid mine count:");
 } else if(Convert.ToInt32 (input) > gridWidth * gridHeight){
 Console.WriteLine ("That is too many mines for this sized grid. Please enter a valid mine count:");
 }
 input = Console.ReadLine ();
 }
 }
 /// <summary>
 /// Takes grid size and number of mines and outputs a multidimentional array
 /// containing Cell values describing the contents of each grid cell.
 /// </summary>
 /// <returns>A Cell[,] array describing a generated minefield.</returns>
 /// <param name="width">The width of the minefield.</param>
 /// <param name="height">The height of the minefield.</param>
 /// <param name="count">The amount of mines contained in the minefield.</param>
 static Cell[,] GenerateMineField(int width, int height, int count) {
 Cell[,] cells = new Cell[width, height];
 // Assign all elements in minefield to empty
 for (int x = 0; x < width; x++) {
 for (int y = 0; y < height; y++) {
 cells [x, y] = Cell.EMPTY;
 }
 }
 Random rnd = new Random ();
 // Assigns final values to each grid
 for (int i = 0; i < count; i++) {
 // Find a random cell to assign a mine
 int x = rnd.Next (0, width);
 int y = rnd.Next (0, height);
 // If the cell is not containing a mine...
 if (cells [x, y] < Cell.MINE) {
 // assign a mine to it
 cells [x, y] = Cell.MINE;
 // For each cell around this mine that isn't also a mine,
 // increase its value by one.
 // After all mines are assigned, this will ensure each cell
 // surrounding the mines are describing how many mines it touches.
 if (y - 1 >= 0) {
 if (cells [x, y - 1] != Cell.MINE) {
 cells [x, y - 1]++;
 }
 }
 if (y + 1 < cells.GetLength(1)) {
 if (cells [x, y + 1] != Cell.MINE) {
 cells [x, y + 1]++;
 }
 }
 if (x - 1 >= 0) {
 if (cells [x - 1, y] != Cell.MINE) {
 cells [x - 1, y]++;
 }
 if (y - 1 >= 0) {
 if (cells [x - 1, y - 1] != Cell.MINE) {
 cells [x - 1, y - 1]++;
 }
 }
 if (y + 1 < cells.GetLength(1)) {
 if (cells [x - 1, y + 1] != Cell.MINE) {
 cells [x - 1, y + 1]++;
 }
 }
 }
 if (x + 1 < cells.GetLength(0)) {
 if (cells [x + 1, y] != Cell.MINE) {
 cells [x + 1, y]++;
 }
 if (y - 1 >= 0) {
 if (cells [x + 1, y - 1] != Cell.MINE) {
 cells [x + 1, y - 1]++;
 }
 }
 if (y + 1 < cells.GetLength(1)) {
 if (cells [x + 1, y + 1] != Cell.MINE) {
 cells [x + 1, y + 1]++;
 }
 }
 }
 // If the randomly selected cell already contains a mine,
 // make the for loop that iteration try again.
 } else {
 i--;
 }
 }
 return cells;
 }
 /// <summary>
 /// Takes a generated minefield and prints it to the console.
 /// </summary>
 /// <param name="field">Cell[,] array containing a generated minefield.</param>
 static void PrintField(Cell[,] field) {
 // The array is technically [height, width], so the for loop
 // traverses the grid in the opposite way to display as
 // the user intended.
 for (int y = 0; y < field.GetLength(1); y++) {
 for (int x = 0; x < field.GetLength(0); x++) {
 if (field [x, y] == Cell.EMPTY) {
 Console.Write (".");
 } else if (field [x, y] == Cell.MINE) {
 Console.Write ("M");
 // If the cell is neither empty nor a mine, display the int value
 // of the Cell enum to show it as the number of mines it touches.
 } else {
 Console.Write (Convert.ToInt32(field [x, y]).ToString());
 }
 }
 // New line
 Console.WriteLine ("");
 }
 }
 }
}

I was unsuccessful in the position and received very direct feedback:

• It's not complete - there is no solver part and existing code is inefficient.
• Incomplete generation algorithm and several other problems present
  1. Code style is ugly (missing brackets everywhere)
  2. Inefficient use of vectors in the initialization (allocating individually, pushing back rather than just iterating)
  3. Unnecessary use of pointers all over the place
  4. Poor algorithm (again as number of mines approaches board size)
  5. Inefficient passing/return by value of vectors all over the place
  6. Ugly decrementing loop counter to force a retry

It's nice to get proper feedback for a change and I understand the decrementing loop issue, however, I'm struggling to line up all the feedback with my code.

Can anyone expand on what each of the feedback points refer to so I understand where I went wrong? Also as an extra, if there any resources you regularly use or are aware of to improve your code practices?

• Pieter Witvoet: While most of the feedback sounds appropriate, 'vector', 'pointer' and 'inefficient passing by value' only make sense to me if this were C++. C# does have pointers and value types, but you're not using pointers, and the C# arrays you're passing around are reference types. Was there any communication about which language(s) you could use?
• Nick Udell: I agree with Pieter's comment, while the code isn't perfect, this feedback doesn't match the right language at all. I'd be hesitant to work under somebody who is going to assume all languages are C++
• t3chb0t: @PieterWitvoet it looks like this feedback is ment to reject any work. Like you've said, it does not make any sense in this context because it does not apply to C#. Maybe OP should have responded that they were wrong and their remarks do not apply here. Or maybe it was even a part of the interview and actually the one that OP failed :-]
• VisualMelon: @t3chb0t interesting theory... I can't see a single missing bracket or brace either. They've even changed the spec (judging by the OP) regarding the lack of solver: dealing with changing requirements/annoying customers? Can't argue with point 6 though; that whole if block could do with some attention.
• Ben Aaronson: Based on that feedback, you dodged a bullet by not getting that position

Answer 1

your GetInput method is a little cluttered. I would create a method that actually takes the input from the console and assigns it directly to the variables for the board.

I created another method called GetInput but it is more flexible than yours. Your GetInput() method was doing too much stuff. let me show you a couple of the methods that I created.

I started with getting the integer because I saw you calling the Integer's TryParse() so many times, you were doing the same thing over and over again, so I made a method:

 public static int GetIntegerFromConsole(string input)
 {
 var output = new int();
 bool conversionSuccess = Int32.TryParse(Console.ReadLine(), out output);
 if (!conversionSuccess)
 {
 Console.WriteLine($"This is not a valid number. Please enter a valid {input}:");
 return 0;
 }
 return output;
 }

it does a little more work for each input, but you only write it once. Now that we have this we can move on to the next method.

This method puts it all together and allows us to give only the information that changes for each command, check it out:

 static void GetInput(string inputRequest, string input, out int inputInteger)
 {
 Console.WriteLine(inputRequest);
 inputInteger = 0;
 while (inputInteger == 0)
 {
 inputInteger = GetIntegerFromConsole(input);
 }
 }

still only one magic string.

now to set the width and the height all I need to do is this:

GetInput("Enter width:", "width", out gridWidth);
GetInput("Enter height:", "height", out gridHeight);

I am not sure how I feel about the Public Static properties/variables or whatever we are calling them...

---

I didn't see any places where brackets were missing, but I did notice that you use two different bracing styles throughout the code, both Java Style and C# style

• Nathan Hall: That's much easier to read without having to add comments! Thank you