patterncsharpMinor
Josephus algorithm using array in C#
Viewed 0 times
josephusalgorithmarrayusing
Problem
I'm trying to write the Josephus algorithm as a project for school, however, I think that there is a better way than what I already have.
Any ideas to make it better?
The output for 10 and 2 is:
Full code:
```
private static int _prisoners, _count;
static void Main()
{
while (true)
{
Console.ForegroundColor = ConsoleColor.White;
GetInitialValues();
int[] lastManAlive = StartTheGame(_prisoners, _count);
Console.ForegroundColor = ConsoleColor.Green;
Console.WriteLine("The last person who is freed is standing at position {0}. ", IsFreed(lastManAlive));
}
}
private static void GetInitialValues()
{
Console.WriteLine("Please enter the number of prisoners that are going to be hanged: ");
try
{
_prisoners = (int) Convert.ToInt64(Console.ReadLine());
}
catch (Exception)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine("Something went wrong, please try again");
GetInitialValues();
}
Console.WriteLine("Please enter a number less than the number of prisoners and greater than 0 to start the game and hang the prisoners: ");
try
{
_count = (int)Convert.ToInt64(Console.ReadLine());
}
catch (Exception)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteL
public static int[] StartTheGame(int prisoners, int count)
{
var position = count - 1;
int[] list = new int[prisoners];
for (int i = 0; i = prisoners)
{
position = (position) - prisoners;
}
if (list[position + 1] == 0)
{
position++;
}
if (list[position + 1] == 1)
{
list[position + 1] = 0;
}
position = position + count + 1;
ShowInConsole(prisoners, list);
}
return list;
}Any ideas to make it better?
The output for 10 and 2 is:
Full code:
```
private static int _prisoners, _count;
static void Main()
{
while (true)
{
Console.ForegroundColor = ConsoleColor.White;
GetInitialValues();
int[] lastManAlive = StartTheGame(_prisoners, _count);
Console.ForegroundColor = ConsoleColor.Green;
Console.WriteLine("The last person who is freed is standing at position {0}. ", IsFreed(lastManAlive));
}
}
private static void GetInitialValues()
{
Console.WriteLine("Please enter the number of prisoners that are going to be hanged: ");
try
{
_prisoners = (int) Convert.ToInt64(Console.ReadLine());
}
catch (Exception)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine("Something went wrong, please try again");
GetInitialValues();
}
Console.WriteLine("Please enter a number less than the number of prisoners and greater than 0 to start the game and hang the prisoners: ");
try
{
_count = (int)Convert.ToInt64(Console.ReadLine());
}
catch (Exception)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteL
Solution
The question is, what is the goal of this task. If it is just return the position of the last man who will survive and your teacher doesn't care about the killing sequence, we can write recursive function like this (source):
Few comments regarding your solution:
Exception
Test your code with initial values 10, 9. Exception occurs - IndexOutOfRange.
I would avoid of method recursive calling in
Improvements
You can generate array with initial value of each element using Enumerable.
If you pass prisoners array to method, then you don't need to know prisoners count. You can simply work with
You display the array in Console in two methods InitiateGame() and ShowInConsole(). Make just one method for this purpose.
Simplify expression from
Use
Method names
Consider use of Array.IndexOf(). Then the function can look like:
Variable names
variable
Parsing, Casting
Paparazzi already asked the question about converting of string to
Your variables are
Alternatives are methods
static void Main(string[] args)
{
int n = 10;
int k = 2;
int position = josephus(n, k);
Console.WriteLine($"The survivor josephus({n},{k}) is {position}.");
}
static int josephus(int n, int k)
{
if (n == 1)
return 1;
else
return (josephus(n - 1, k) + k - 1) % n + 1;
}Few comments regarding your solution:
Exception
Test your code with initial values 10, 9. Exception occurs - IndexOutOfRange.
I would avoid of method recursive calling in
catch block in GetInitialValues(). It could cause unexpected behavior of your application.Improvements
You can generate array with initial value of each element using Enumerable.
int[] prisoners = Enumerable.Repeat(1, prisonersCount).ToArray();If you pass prisoners array to method, then you don't need to know prisoners count. You can simply work with
prisoners.Length property.You display the array in Console in two methods InitiateGame() and ShowInConsole(). Make just one method for this purpose.
Simplify expression from
position = (position) - prisoners; to position -= prisoners;Use
foreach cycle if you don't need change array members. Simplify condition with ternary operator. You can use string interpolation when you format strings.public static void ShowInConsole(int[] prisoners)
{
Console.WriteLine();
foreach (int prisoner in prisoners)
{
Console.ForegroundColor = (prisoner == 0) ? ConsoleColor.Red : ConsoleColor.Green;
Console.Write($" {prisoner} ");
}
Console.WriteLine();
}Method names
GetInitialValues() is void. Get says that method should return some initial values. The other case is StartTheGame() which returns int[] array. That is strange. IsFreed() returns int. Usually if I see method named like IsSomeAdjective(), I usually expect boolean return type.Consider use of Array.IndexOf(). Then the function can look like:
public static int GetFreedPosition(int[] prisoners)
{
return Array.IndexOf(prisoners, 1) + 1;
}Variable names
variable
list is in reality array int[]. I would call it prisoners. And the prisoners in fact represents prisoners count, so why don't call it prisonersCount.Parsing, Casting
Paparazzi already asked the question about converting of string to
Int64.Your variables are
System.Int32 structures, so you don't need to convert to Int64 and then explicitly cast to Int32. Simply use Convert.ToInt32(Console.ReadLine()).Alternatives are methods
int.Parse() or int.TryParse().Code Snippets
static void Main(string[] args)
{
int n = 10;
int k = 2;
int position = josephus(n, k);
Console.WriteLine($"The survivor josephus({n},{k}) is {position}.");
}
static int josephus(int n, int k)
{
if (n == 1)
return 1;
else
return (josephus(n - 1, k) + k - 1) % n + 1;
}int[] prisoners = Enumerable.Repeat(1, prisonersCount).ToArray();public static void ShowInConsole(int[] prisoners)
{
Console.WriteLine();
foreach (int prisoner in prisoners)
{
Console.ForegroundColor = (prisoner == 0) ? ConsoleColor.Red : ConsoleColor.Green;
Console.Write($" {prisoner} ");
}
Console.WriteLine();
}public static int GetFreedPosition(int[] prisoners)
{
return Array.IndexOf(prisoners, 1) + 1;
}Context
StackExchange Code Review Q#148577, answer score: 2
Revisions (0)
No revisions yet.