patterncsharpMinor

Climbing the stairs

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

stairstheclimbing

Problem

A quick description on the problem I solved on codewars.com but remember I just want a small part of my solution refactored (just the for-loop):

Description

Vasya wants to climb up a stair of certain amount of steps (Input
parameter 1). There are 2 simple rules that he has to stick to.

Vasya can climb 1 or 2 steps at each move. Vasya wants the number of
moves to be a multiple of a certain integer. (Input parameter 2).
Task:

What is the MINIMAL number of moves making him climb to the top of the
stairs that satisfies his conditions?

Input

Number of stairs: 0 < N ≤ 10000 ; Integer to be multiplied : 1 < M ≤
10; Output

Return a single integer - the minimal number of moves being a multiple
of M; If there is no way he can climb satisfying condition return - 1
instead.

This is my C# method and I only want to refactor the for-loop on line 13:

private static int NumberOfSteps(int n , int m)
    {
        Console.WriteLine(n + "    " + m);
        if (m  query = Enumerable.Range(1, TwoSteps).Select(x => x = 2).ToList();
        List Onesteps = Enumerable.Range(1, n - query.Sum()).Select(x => x = 1).ToList();
        query.AddRange(Onesteps);
        if (query.Count() % m == 0) { return query.Count(); }

        for (var count = 0; count () { 1, 1 });
                if (query.Count() % m == 0) { Console.WriteLine(query.Count()); return query.Count(); }
            }
            if (query.Count() % m == 0) { return query.Count(); }
        }
        Console.WriteLine(-1);
        return -1;
    }

This is part of my code I would like to know how I could refactor it into a Linq Method Syntax statement:

for (var count = 0; count () { 1, 1 });
            if (query.Count() % m == 0) { Console.WriteLine(query.Count()); return query.Count(); }
        }
        if (query.Count() % m == 0) { return query.Count(); }
    }

Solution

The code below is my C# Method and I only want to refactor the for-loop on line 13 I think.

Sorry but you won't only get this, because your code has a large number of problems.

Let us begin at the top of your code and review it to the bottom.

Return a single integer - the minimal number of moves being a multiple of M; If there is no way he can climb satisfying condition return - 1 instead.

Why bothering with printing to the Console if you only need to return an integer ?

Your method should have a clear responsibility which is returning the minimal numbers of moves. Printing some values to the Console doesn't belong here.

The description of the problem clearly states that the number of stairs are in the range 0 will take an IEnumerable as a parameter. You don't really need the OneSteps variable and the code could be changed like so

query.AddRange(Enumerable.Repeat(1, n - (amountOfTwoSteps * 2)));

You are using the Count() method very often, which involves for a List a null check, a soft cast via as to ICollection and again a null check

 each time.  

From the reference source

public static int Count(this IEnumerable source) {
        if (source == null) throw Error.ArgumentNull("source");
        ICollection collectionoft = source as ICollection;
        if (collectionoft != null) return collectionoft.Count;
        ....
        ....


If you know that the type you use is an implementation of an

ICollection, like the List you should consider to just access the Count

 property.  

if (query[count] == 2)
{
    query.RemoveAt(count);
    query.AddRange(new List() { 1, 1 });
    if (query.Count() % m == 0) { Console.WriteLine(query.Count()); return query.Count(); }
}
if (query.Count() % m == 0) { return query.Count(); }


If we look at the body of your

for

 loop, we can see that you have some duplicated code which can be removed. I don't see a reason to remove an item of the

List, create a new List and use AddRange to add this List to query every time. This could be simplified by setting the value of the current item to 1 and adding a 1` to the list like so

if (query[count] == 2)
{
    query[count] = 1;
    query.Add(1);
}
if (query.Count % m == 0) { return query.Count; }

Putting all together will lead to

private static int NumberOfSteps(int n, int m)
{
    if (m  10) { throw new ArgumentOutOfRangeException("m"); }
    if (n  10000) { throw new ArgumentOutOfRangeException("n"); }

    var amountOfTwoSteps = n / 2;

    if (amountOfTwoSteps % m != 0)
    {
        amountOfTwoSteps--;
    }

    var query = Enumerable.Repeat(2, amountOfTwoSteps).ToList();
    query.AddRange(Enumerable.Repeat(1, n - (amountOfTwoSteps * 2)));

    if (query.Count % m == 0) { return query.Count; }

    for (var count = 0; count <= query.Count - 1; count++)
    {
        if (query[count] == 2)
        {
            query[count] = 1;
            query.Add(1);
        }
        if (query.Count % m == 0) { return query.Count; }
    }

    return -1;
}

which still has some magic numbers in it which should be extracted to some meaningful named constants which is for you to do.

Code Snippets

var TwoSteps = Math.Ceiling(((double)n / 2)) % m != 0 ? ((n / 2) - 1) : (n / 2);

var amountOfTwoSteps = n / 2;

if (amountOfTwoSteps % m != 0)
{
    amountOfTwoSteps--;
}

List<int> query = Enumerable.Range(1, TwoSteps).Select(x => x = 2).ToList();

var query = Enumerable.Repeat(2, amountOfTwoSteps).ToList();

List<int> Onesteps = Enumerable.Range(1, n - query.Sum()).Select(x => x = 1).ToList();

Context

StackExchange Code Review Q#151490, answer score: 9

Revisions (0)

No revisions yet.