2
\$\begingroup\$

Example:
if \$N=2\$, given the input array \${[1, 2, 3, 4, 5, 6]}\$ the function should return \${[5, 6, 1, 2, 3, 4]}\$

Here is my code:

def copyDigit(list,index,item):

        """
                Copy the item to the indexed location in the given list
        """
        list[index] = item
        return list

def rotateList(noListToRorate,timesRotate):

        """
                Rotate List to right    
        """
        print "Function received {0},{1}".format(noListToRorate,timesRotate)
        for rotate in range(N):
            lastItemInList = noListToRorate[-1]
            for i in range(len(noListToRorate)-2,-1,-1):
                itemToShift = noListToRorate[i]
                noListToRorate = copyDigit(noListToRorate,i+1,itemToShift)
            noListToRorate[0] = lastItemInList
        print "Rotate once: {0}".format(noListToRorate)
        return noListToRorate


if __name__ == '__main__':

        """
           This program will rorate right the given list N no of times  
        """

        arrayList = [1,2,3,4,5,6]
        N = 2
        print "Rotate an array: " ,(arrayList), "No of times: ", N
        finalList = rotateList(arrayList,N)
        print "Rotated List: ", finalList
\$\endgroup\$
8
\$\begingroup\$

The obvious solution

Not sure if you know the obvious solution and just want to re-invent the wheel or if you are just not aware of this but you could just do :

>>> l = [1, 2, 3, 4, 5, 6]
>>> n = 2
>>> l[-n:] + l[:-n]
[5, 6, 1, 2, 3, 4]

The actual review : details

You have split your code into multiple functions, documented them, and written the usual if __name-- == 'main': test : this is already a pretty good beginning.

Also, pep8 only complies about some whitespace not following the Python Style Guide (this is worth fixing it anyway).

However, here a few things that do not look good in your code and could easily be fixed :

  • your code can be python 3 compliant by adding parentheses in your calls to print.
  • you have the same typo rorate in multiple places
  • list is a bad name for a list as you can easily mix-up with the type
  • your function and variable names do not follow the convention.

After fixing all this, you get something like :

def copy_digit(lst, index, item):
    """
            Copy the item to the indexed location in the given list
    """
    lst[index] = item
    return lst


def rotate_list(lst, nb_rotate):
    """
            Rotate List to right
    """
    print("Function received {0},{1}".format(lst, nb_rotate))
    for rotate in range(N):
        last_item = lst[-1]
        for i in range(len(lst) - 2, -1, -1):
            item_to_shift = lst[i]
            lst = copy_digit(lst, i + 1, item_to_shift)
        lst[0] = last_item
    print("Rotate once: {0}".format(lst))
    return lst


if __name__ == '__main__':
    """
       This program will rotate right the given list N no of times
    """

    array = [1, 2, 3, 4, 5, 6]
    N = 2
    print("Rotate an array: ", array, "No of times: ", N)
    final_list = rotate_list(array, N)
    print("Rotated List: ", final_list)

A subtle bug

What if I was to call :

final_list = rotate_list(array, 2)

? Surprisingly enough, I'd get an error. I'll let you have a look at this because it is not very complicated.

Clearer interface for your functions

Your function copy_digit updates the list and returns it. There is no real need for this as the caller would already have the initial list. Then you can just have :

def copy_digit(lst, index, item):
    """
            Copy the item to the indexed location in the given list
    """
    lst[index] = item

and

...
copy_digit(lst, i + 1, item_to_shift)
...

Of course, the need for a function seems a bit doubtful here. Let's inline this :

...
lst[i + 1] = item_to_shift
...

Then, it seems like the item_to_shift variable is not really required anymore :

for i in range(len(lst) - 2, -1, -1):
    lst[i + 1] = lst[i]

And more generally

In Programming Pearls (sorry I couldn't find the relevant full text), Jon Bentley describes different solutions to the problem. Some are quite original and actually very easy to implement on top of being efficient. I'll let you google "programming pearls rotate" if you want to read more about this. It is definitely worth a read (and the whole book is too).

\$\endgroup\$
  • \$\begingroup\$ Thanks a ton for a detailed review. I am learning on so not that great in naming the variables and functions according to the standards. Your link to PEP8 is definitely useful. Yes, I am aware of the slicing concept to solve this, which is one liner but just wanted to write another program. Could I install PEP8 in windows too ? \$\endgroup\$ – python_lover Oct 20 '14 at 17:54
  • \$\begingroup\$ Subtle error do you mean you mean the handling of conditions N=0 and N > array length, which is missing. \$\endgroup\$ – python_lover Oct 20 '14 at 18:05
  • \$\begingroup\$ I meant : removing the declaration of n and passing the value directly to the function. \$\endgroup\$ – Josay Oct 20 '14 at 19:24
  • \$\begingroup\$ I am not getting an error in my existing code even when passing 2 as the argument to the function. ?? \$\endgroup\$ – python_lover Oct 21 '14 at 4:46
  • \$\begingroup\$ Unable to install pip in my windows with python3.0, any help? I have chocolatey but neither able to install pip nor easy.install. Any help?? \$\endgroup\$ – python_lover Oct 21 '14 at 4:48
0
\$\begingroup\$

Another not so obvious solution

Compute for each element of the array, it's final position in the rotated array and simply move it there.

Given a list of \$N\$ items, and we are to move all the elements to the right \$X\$ times, we can compute the final position \$p_i\$ of element \$e_i\$ by doing:

\$p_i = (i + X) \bmod N\$

Create a function

def rotate_list(lst, x):
    """
            Rotate List to right
    """
    length = len(lst)
    rotated_list = [0] * length

    for i, e in enumerate(lst):
        p = (i + x) % length
        rotated_list[p] = e
    return rotated_list

Demonstration

To test this, you can pass in the array \${[1, 2, 3, 4, 5, 6]}\$ with \$N=2\$, to confirm that it does indeed give \${[5, 6, 1, 2, 3, 4]}\$

More review

To make your program reusable, consider making it take arguments from the command line. So your main code should be:

import sys
import ast
if __name__ == '__main__':
    """
        This program will rotate right the given list N no of times  
    """
    if len(sys.argv) < 3:
        print ("Usage: %s \"[a, b, c, d,...]\" N" % sys.argv[0])
        quit(1)

    arrayList = ast.literal_eval(sys.argv[1])
    N = int(sys.argv[2])


    print ("Rotate an array: " ,(arrayList), "No of times: ", N)
    finalList = rotate_list(arrayList,N)
    print ("Rotated List: ", finalList)
\$\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.