Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am a biologist. Some time ago, I wanted to learn to program, and since I am fascinated by the 4th dimension and I was fascinated by the rotatation of hypercubes and hypersphere, so I decided to understand it better, and thanks to the work of Steve Hollasch, I understood that humans cannot "see" the 4th dimension, only project it in the 3 dimensional world.

Anyway, after finding the function to project the 4th dimension, I wanted to do something interactive with it, something where one can learn where and how the object moves in a 4 dimensional space. So I came up with the idea of a simple game: Snake.

snake4d

Can you please comment on the code here?

__author__ = "Mauro Pellanda"
__credits__ = ["Mauro Pellanda"]
__license__ = "GNU"
__version__ = "1.1.0"
__maintainer__ = "Mauro Pellanda"
__email__ = "pmauro@ethz.ch"
__status__ = "Devlopment"


''' In this file is contained the snake definition'''

def get_random_color():
    return "#%02x%02x%02x" % (random.randrange(0,255), random.randrange(0,255), random.randrange(0,255))

from vec import *
import time


class Snake:

    def __init__(self):
        self.head_pos = V4(0,0,0,0) #position of the head
        self.head_dir ="UP" #direction of the head
        self.p_list = [] #store the polygon of the snake
        self.score = 0 #score
        self.score2 = 0 #number of cube taken
        self.time = 0. #boh
        self.color = [0, 255, 0] #color
        self.color_var = "up" #Variable to circulary change color


    def create_cube(self,point):
        '''This function return the polygon coordinates in a
        determined postion defined by point'''
        #calculate coordinates
        v = V4(.4,.4,.4,.4)
        higher = sum4(point,v)
        lower = sub4(point,v)
        c = cube4d(lower,higher)
        #calculate the color
        if self.color_var == "up":          
            self.color[0] += 20
            if self.color[0] > 255-21:
                self.color_var = "down"
        elif self.color_var == "down":
            self.color[0] -= 20
            if self.color[0] < 21:
                self.color_var = "up"           
        color_tuple = (self.color[0],self.color[1],self.color[2])
        c.color = "#%02x%02x%02x" % color_tuple
        #add the tag for the canvas (not used in this version)
        c.tag = "snake"
        return c

    def initialize_snake(self):
        '''it initialize the snake at the original position with 4 cubes'''
        self.__init__()
        size = 4
        for x in range(size+1):
            point = V4(0,-(size-x),0,0)
            self.p_list.append(self.create_cube(point))

    def move(self,dir):
        '''check if is a valid move in that direction, the snake cannot go
        in opposite direction'''
        no_move = False
        if dir == "UP" and self.head_dir != "DOWN":
            dir_v = V4(0,1,0,0)
        elif dir == "DOWN" and self.head_dir != "UP":
            dir_v = V4(0,-1,0,0)
        elif dir == "LEFT" and self.head_dir != "RIGHT":
            dir_v = V4(-1,0,0,0)
        elif dir == "RIGHT" and self.head_dir != "LEFT":
            dir_v = V4(1,0,0,0)
        elif dir == "FW" and self.head_dir != "RW":
            dir_v = V4(0,0,1,0)
        elif dir == "RW" and self.head_dir != "FW":
            dir_v = V4(0,0,-1,0)
        elif dir == "IN" and self.head_dir != "OUT":
            dir_v = V4(0,0,0,-1)
        elif dir == "OUT" and self.head_dir != "IN":
            dir_v = V4(0,0,0,1)
        else:
            no_move = True

        if not no_move:
            #move the snake, and append a new polygon in the new position,
            #take off the polygon from the tail, the snake is stored in the
            #list like this: [tail,->,head]
            self.head_pos = sum4(self.head_pos, dir_v)
            self.head_dir = dir
            self.p_list.pop(0)
            self.p_list.append(self.create_cube(self.head_pos))
  1. Are the comments useful to understand what I did?
  2. Do you have any suggestions about the programming style and/or the algorithms?

Here is the download of the complete source.

share|improve this question

1 Answer 1

up vote 7 down vote accepted
__author__ = "Mauro Pellanda"
__credits__ = ["Mauro Pellanda"]
__license__ = "GNU"
__version__ = "1.1.0"
__maintainer__ = "Mauro Pellanda"
__email__ = "pmauro@ethz.ch"
__status__ = "Devlopment"


''' In this file is contained the snake definition'''

For this to be recognized as a docstring I think it needs to be the first thing in a file.

def get_random_color():
    return "#%02x%02x%02x" % (random.randrange(0,255), random.randrange(0,255), random.randrange(0,255))

from vec import *
import time

imports are typically placed before function definitions

class Snake:

If you are working python 2.x, I recommend you make your classes inherit from object. That way they are new style classes.

    def __init__(self):
        self.head_pos = V4(0,0,0,0) #position of the head
        self.head_dir ="UP" #direction of the head

Using a string for something like this is a little odd. I suggest a boolean value or an integer.

        self.p_list = [] #store the polygon of the snake
        self.score = 0 #score
        self.score2 = 0 #number of cube taken
        self.time = 0. #boh
        self.color = [0, 255, 0] #color

I'd make color a tuple. Lists are for storing, well lists, not related items like colors. I know you do this so you can modify it. But I'd create new tuples everytime I changed the color.

        self.color_var = "up" #Variable to circulary change color

Agains strings are odd unless its actually text. It'll take slightly longer to compare strings then say ints or bools.

    def create_cube(self,point):
        '''This function return the polygon coordinates in a
        determined postion defined by point'''

Typically, the first ''' and final ''' are placed on a line by themselves for multiline docstrings

        #calculate coordinates
        v = V4(.4,.4,.4,.4)
        higher = sum4(point,v)
        lower = sub4(point,v)
        c = cube4d(lower,higher)
        #calculate the color
        if self.color_var == "up":          
            self.color[0] += 20
            if self.color[0] > 255-21:
                self.color_var = "down"
        elif self.color_var == "down":
            self.color[0] -= 20
            if self.color[0] < 21:
                self.color_var = "up"           
        color_tuple = (self.color[0],self.color[1],self.color[2])

You can just do color_tuple = tuple(self.color)

        c.color = "#%02x%02x%02x" % color_tuple

You've duplicated the tuple to string logic here and in the random color function. I suggest refactoring so they share it.

        #add the tag for the canvas (not used in this version)
        c.tag = "snake"
        return c

    def initialize_snake(self):
        '''it initialize the snake at the original position with 4 cubes'''
        self.__init__()

Don't ever do that. __init__ is called when the object is built. You shouldn't call it later. Python will let you, but nobody expects you to do it and you are liable to confuse people.

        size = 4
        for x in range(size+1):
            point = V4(0,-(size-x),0,0)
            self.p_list.append(self.create_cube(point))

    def move(self,dir):
        '''check if is a valid move in that direction, the snake cannot go
        in opposite direction'''
        no_move = False
        if dir == "UP" and self.head_dir != "DOWN":
            dir_v = V4(0,1,0,0)
        elif dir == "DOWN" and self.head_dir != "UP":
            dir_v = V4(0,-1,0,0)
        elif dir == "LEFT" and self.head_dir != "RIGHT":
            dir_v = V4(-1,0,0,0)
        elif dir == "RIGHT" and self.head_dir != "LEFT":
            dir_v = V4(1,0,0,0)
        elif dir == "FW" and self.head_dir != "RW":
            dir_v = V4(0,0,1,0)
        elif dir == "RW" and self.head_dir != "FW":
            dir_v = V4(0,0,-1,0)
        elif dir == "IN" and self.head_dir != "OUT":
            dir_v = V4(0,0,0,-1)
        elif dir == "OUT" and self.head_dir != "IN":
            dir_v = V4(0,0,0,1)
        else:
            no_move = True

Chains of elifs are a sign to look for a better solution. For example, you could have a dictionary mapping directionts to opposites and vectors. Then you'd just have to fetch from the dictionary.

        if not no_move:

Double negative, consider inverting the logic on no_move so it becomes moved.

            #move the snake, and append a new polygon in the new position,
            #take off the polygon from the tail, the snake is stored in the
            #list like this: [tail,->,head]
            self.head_pos = sum4(self.head_pos, dir_v)
            self.head_dir = dir
            self.p_list.pop(0)
            self.p_list.append(self.create_cube(self.head_pos))
share|improve this answer
    
Thank you very much for the comments!! A specific question about the elif chain, can I define something like this dic_opp_dir = {"UP":"DOWN", ecc} then call def catch_dir(dir_x): for dir in dic: if dir_x == dic[x]: move = false ? –  Pella86 Sep 16 '11 at 9:34
    
And there are enum in python? in C++ I defined the directions as enum. And is better then a string, because giving an int decrease the readability of the code... –  Pella86 Sep 16 '11 at 9:34
1  
@Pella86, why are you wanting to put a loop in that example? Just check if dir_x == dic_opp_dir[head_x]. Python doesn't have enum, but I'd add global constants UP = 0 and DOWN = 1 and use those. –  Winston Ewert Sep 16 '11 at 12:18
    
I didn't know such things can be done with a dictionary... I almost never use them (even though I know they are really useful). And about the global constat I thought it too, but maybe there was a better way. Thank you! –  Pella86 Sep 16 '11 at 12:25
    
You wrote to never call self.__init__() I usually don't do this, but I need it to reset all varibles, there is a nicer way to do it instead of copying the init funtion in the initialize_snake? –  Pella86 Sep 17 '11 at 16:26

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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