patterncsharpMinor
Optimizing Bingo Caller Method
Viewed 0 times
calleroptimizingbingomethod
Problem
I am dong a competitive programming problem I have passed all the tests.
but my program times out. I need help optimizing my code so it can run faster.
Task:
-
Finish method:
-
Return all numbers in the range of 1 until 75 once and in random order
-
If there are no numbers left, return an empty string
-
The numbers are returned one by one in Bingo style:
-
These are the ranges that you must follow:
-
A number within range 1 to 15 starts with a 'B'
-
A number within range 16 to 30 starts with an 'I'
-
A number within range 31 to 45 starts with an 'N'
-
A number within range 46 to 60 starts with a 'G'
-
A number within range 61 to 75 starts with an 'O'
-
Use
My code below
```
using System;
using System.Collections.Generic;
public class BingoCaller
{
private Random random;
static List List = new List();
public BingoCaller(Random random)
{
this.random = random;
}
public string GetNumber()
{
Start:
var item = random.Next(75);
if (List.Contains(item))
{
goto Start;
}
else
{
List.Add(item);
if (item >= 61)
{
return "O" + item;
}
else if (item >= 46)
{
return "G" + item;
}
else if (item >= 31)
{
return "N" + item;
}
else if (item >= 16)
{
but my program times out. I need help optimizing my code so it can run faster.
Task:
-
Finish method:
BingoCaller.GetNumber()-
Return all numbers in the range of 1 until 75 once and in random order
-
If there are no numbers left, return an empty string
-
The numbers are returned one by one in Bingo style:
"I27", "N40", "B5", "B12", "I28", "O69", "B1", ...-
These are the ranges that you must follow:
-
A number within range 1 to 15 starts with a 'B'
-
A number within range 16 to 30 starts with an 'I'
-
A number within range 31 to 45 starts with an 'N'
-
A number within range 46 to 60 starts with a 'G'
-
A number within range 61 to 75 starts with an 'O'
-
Use
System.Random to generate your random numbers. Pass via the constructor for testing purposes.My code below
```
using System;
using System.Collections.Generic;
public class BingoCaller
{
private Random random;
static List List = new List();
public BingoCaller(Random random)
{
this.random = random;
}
public string GetNumber()
{
Start:
var item = random.Next(75);
if (List.Contains(item))
{
goto Start;
}
else
{
List.Add(item);
if (item >= 61)
{
return "O" + item;
}
else if (item >= 46)
{
return "G" + item;
}
else if (item >= 31)
{
return "N" + item;
}
else if (item >= 16)
{
Solution
It is considered bad practice to use goto-statements in modern programming because it quickly gets hard to follow the structure, so avoid that. Instead you could use a while-loop in a kind of this manner:
This statement sets
instead of
It seems that
Instead of the long list of if-statements, you could use the simple relation between the prefix's position in the word "BINGO" and the random number to extract it as:
(it has something to do with 15).
The pattern of filling a list with found numbers tends to be slower and slower in response time as the size of the list grows because the next random number is more and more likely to be in the list. Potentially a valid number may never be found...
A better approach could be to create the list of the 75 numbers in the constructor in advance and then remove them one by one from the list when found by
This will decrease in response time and you avoid the looping.
A solution could be:
while (list.Count < 75)
{
var number = random.Next(75);
if (list.Contains(number))
continue;
...
}var item = random.Next(75);This statement sets
item to a number in the interval of \$[0, 75[\$, but you were supposed to handle the interval \$[1, 75]\$instead of
if (item >= 61) it is more readable to write if (item > 60)It seems that
GetNumber() is missing a stop-condition. It will run forever between goto Start and Start: when the list is full. (That may be your "time out"-problem).Instead of the long list of if-statements, you could use the simple relation between the prefix's position in the word "BINGO" and the random number to extract it as:
string bingo = "BINGO"; // Make it a class field.
int index = ...
char prefix = bingo[index](it has something to do with 15).
The pattern of filling a list with found numbers tends to be slower and slower in response time as the size of the list grows because the next random number is more and more likely to be in the list. Potentially a valid number may never be found...
A better approach could be to create the list of the 75 numbers in the constructor in advance and then remove them one by one from the list when found by
random.Next():...
var number = list[random.Next(0, list.Count)];
list.Remove(number);
...This will decrease in response time and you avoid the looping.
A solution could be:
public class BingoCaller
{
private Random random;
List list;
string bingo = "BINGO";
public BingoCaller(Random random)
{
this.random = random;
list = Enumerable.Range(1, 75).ToList();
}
public string GetNumber()
{
if (list.Count == 0)
return "";
var number = list[random.Next(0, list.Count)];
list.Remove(number);
char prefix = bingo[(number - 1) / 15];
return $"{prefix}{number}";
}
}Code Snippets
while (list.Count < 75)
{
var number = random.Next(75);
if (list.Contains(number))
continue;
...
}var item = random.Next(75);string bingo = "BINGO"; // Make it a class field.
int index = ...
char prefix = bingo[index]...
var number = list[random.Next(0, list.Count)];
list.Remove(number);
...public class BingoCaller
{
private Random random;
List<int> list;
string bingo = "BINGO";
public BingoCaller(Random random)
{
this.random = random;
list = Enumerable.Range(1, 75).ToList();
}
public string GetNumber()
{
if (list.Count == 0)
return "";
var number = list[random.Next(0, list.Count)];
list.Remove(number);
char prefix = bingo[(number - 1) / 15];
return $"{prefix}{number}";
}
}Context
StackExchange Code Review Q#155134, answer score: 7
Revisions (0)
No revisions yet.