2
\$\begingroup\$

I'm learning C#, I programmed some things in java, and I would like to have a feedback on my C# code.
The exercise is from codility.

The goal is to rotate array A K times; that is, each element of A will be shifted to the right K times.

Write a function:

class Solution { public int[] solution(int[] A, int K); }

that, given an array A consisting of N integers and an integer K, returns the array A rotated K times.

For example, given

A = [3, 8, 9, 7, 6]
K = 3 the function should return [9, 7, 6, 3, 8]. Three rotations were made:

[3, 8, 9, 7, 6] -> [6, 3, 8, 9, 7]
[6, 3, 8, 9, 7] -> [7, 6, 3, 8, 9]
[7, 6, 3, 8, 9] -> [9, 7, 6, 3, 8] For another example, given

A = [0, 0, 0]
K = 1 the function should return [0, 0, 0]

Given

A = [1, 2, 3, 4]
K = 4 the function should return [1, 2, 3, 4]


And this is my code. It gives 100% but maybe something can be improved.Thanks in advice.

using System;
// you can also use other imports, for example:
// using System.Collections.Generic;

// you can write to stdout for debugging purposes, e.g.
// Console.WriteLine("this is a debug message");

class Solution {
    public int[] solution(int[] A, int K) {
        int len = A.Length;
        int[] B = new int[len];
        if(len > 0 && K % len != 0 )
        {
            for ( int i = 0; i < len; i++)
            {
                B[(K + i) % len] = A[i];
            }
        }
        else
        {
            return A;
        }
        return B;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Now that I'm looking at it, I believe B could be created inside the if... so if no cyclic rotation is needed computer could save the creation of a potentially large array \$\endgroup\$ – newbie Sep 20 '19 at 12:34
3
\$\begingroup\$

Good Practices

  • ✔ I like the fact you don't rotate if the array's length is a multiplication of K.
  • ✔ As you stated in the comments, the second array B gets created too early, since in some occasions it would be created in vain.

Review

  • The challenge is clearly written for Java:class Solution { public int[] solution(int[] A, int K); }. The provided signature deserves its own review (casing conventions, static members, meaningful member names). However, if you 'cannot' change the signature given to you, I would be consistent and use N instead of len for the array's length.
  • You sometimes return the provided array and sometimes create a new one. I would make this very clear in the specification. A consumer should know whether or when a copy is returned. This affects further usage of the object.
  • You should guard against the source array being null.
  • How would you handle a negative value of K? If you think about it, a left rotation is just the inverse of a right rotation.
  • Nested statements could have been prevented by inverting the if-statement and exiting early. Less nested statements generally increase readability.
  • For bigger arrays you should check Buffer.BlockCopy to optimize copying sections of an array to another array. You could see a rotation as copying 2 sections from one array to another given a pivot. Check out this article about the subject.
  • If you can change the signature, make a static class with static method, use a meaningful method name RotateRight with arguments array and count and make it clear whether in-place rotation is performed or a new array is created.
|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.