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

Josephus algorithm using array in C#

Submitted by: @import:stackexchange-codereview··
0
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.

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):

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.