3
\$\begingroup\$

I have made a Rubiks Cube object which builds a Cube out of Cubelet objects and then allows you to move it. Things I still plan to do is add tower cubes and triangles and other shapes as well as a method to save and load files and a graphics program to use it however I decided it was probably done enough to see what people thought about it.

#0: Red, 1: White, 2: Green, 3: Orange, 4: Yellow, 5: Blue
from copy import deepcopy

class Cubelet:
    def __init__(self, up=None, left=None, front=None, down=None, right=None,
                 back=None, patterned = False):
        self.up = up
        self.left = left
        self.front = front
        self.down = down
        self.right = right
        self.back = back
        self.patterned = patterned
    def __str__(self):
        message = ""
        if self.up != None: message += "up: " + str(self.up) + ", "
        if self.left != None: message += "left: " + str(self.left) + ", "
        if self.front != None: message += "front: " + str(self.front) + ", "
        if self.down != None: message += "down: " + str(self.down) + ", "
        if self.right != None: message += "right: " + str(self.right) + ", "
        if self.back != None: message += "back: " + str(self.back) + ", "
        if message == "": message = "no attributes  "
        return("Cubelet object with " + message[:-2])
    def rotate(self, x=0, y=0, z=0):
        for i in range(x%4):
            up, front, down, back = self.up, self.front, self.down, self.back
            self.up, self.front, self.down, self.back = front, down, back, up
            if self.patterned:
                sides = ["up", "back", "down", "front"]
                if type(self.left) == tuple:
                    self.left = self.left[0], sides[(sides.index(self.left[1])+1)%4]
                if type(self.right) == tuple:
                    self.right = self.right[0], sides[(sides.index(self.right[1])+3)%4]
        for i in range(y%4):
            front, left, back, right = self.front, self.left, self.back, self.right
            self.front, self.left, self.back, self.right = right, front, left, back
            if self.patterned:
                sides = ["left", "back", "right", "front"]
                if type(self.up) == tuple:
                    self.up = self.up[0], sides[(sides.index(self.up[1])+1)%4]
                if type(self.down) == tuple:
                    self.down = self.down[0], sides[(sides.index(self.down[1])+3)%4]
        for i in range(z%4):
            up, right, down, left = self.up, self.right, self.down, self.left
            self.up, self.right, self.down, self.left = left, up, right, down
            if self.patterned:
                sides = ["left", "up", "right", "down"]
                if type(self.front) == tuple:
                    self.front = self.front[0], sides[(sides.index(self.front[1])+1)%4]
                if type(self.back) == tuple:
                    self.back = self.back[0], sides[(sides.index(self.back[1])+3)%4]
        return self

class Cube:
    def __init__(self, form="3"):
        if form[-1] == "p":
            self.patterned = True
            self.size = int(form[:-1])
        else:
            self.patterned = False
            self.size = int(form)
        self.form = form
        self.cube = [[[] for i in range(self.size)] for j in range(self.size)]
        if self.patterned: side_values = [(1, "front"), (4, "back"), (5, "up"),
                                     (2, "down"), (0, "left"), (3, "right")]
        else: side_values = [1, 4, 5, 2, 0, 3]
        for y in range(self.size):
            for z in range(self.size):
                for x in range(self.size):
                    cubelet = Cubelet()
                    if x == 0: cubelet.left=side_values[0]
                    if x == self.size-1: cubelet.right=side_values[1]
                    if z == 0: cubelet.back=side_values[2]
                    if z == self.size-1: cubelet.front=side_values[3]
                    if y == 0: cubelet.up=side_values[4]
                    if y == self.size-1: cubelet.down=side_values[5]
                    self.cube[y][z].append(cubelet)
        self.set_faces()
        self.moves = 0
    def __getitem__(self, index):
        return(self.cube[index[0]][index[1]][index[2]])
    def __setitem__(self, index, value):
        self.cube[index[0]][index[1]][index[2]] = value
        self.set_faces()
    def __str__(self):
        if self.patterned: return("A size " + str(self.size) + " patterned cube")
        else: return("A size " + str(self.size) + " cube")
    def set_faces(self):
        self.up = [[self[0,z,x].up for x in range(self.size)] for z in range(self.size)]
        self.left = [[self[y,z,0].left for z in range(self.size)] for y in
                        range(self.size)]
        self.front = [[self[y,self.size-1,x].front for x in range(self.size)] for y in
                        range(self.size)]
        self.down = [[self[self.size-1,z,x].down for x in range(self.size-1,-1,-1)] for z in
                        range(self.size-1,-1,-1)]
        self.right = [[self[y,z,self.size-1].right for z in range(self.size-1,-1,-1)] for y in
                        range(self.size-1,-1,-1)]
        self.back = [[self[y,0,x].back for x in range(self.size-1,-1,-1)] for y in
                        range(self.size-1,-1,-1)]
    def move_side(self, side, amount=1, depth=1):
        for i in range(amount%4):
            copy = deepcopy(self)
            for j in range(self.size):
                for k in range(self.size):
                    if side == "front":
                        self[k,self.size-depth,j] = copy[self.size-1-j,self.size-depth,k].rotate(z=1)
                    elif side == "back":
                        self[k,depth-1,j] = copy[j,depth-1,self.size-1-k].rotate(z=-1)
                    elif side == "right":
                        self[k,j,self.size-depth] = copy[j,self.size-1-k,self.size-depth].rotate(x=1)
                    elif side == "left":
                        self[k,j,depth-1] = copy[self.size-1-j,k,depth-1].rotate(x=-1)
                    elif side == "up":
                        self[depth-1,k,j] = copy[depth-1,j,self.size-1-k].rotate(y=-1)
                    elif side == "down":
                        self[self.size-depth,k,j] = copy[self.size-depth,self.size-1-j,k].rotate(y=1)
        self.set_faces()
        self.moves += 1

Possible way of using this is:

c = Cube("4") #Creates a 4x4x4 Rubiks Cube
cp = Cube("3p") #Creates a 3x3x3 patterned Rubiks Cube (each cubelet face is a vector rather than scalar)
c.move_side("front", depth=2) #moves the face one layer back from the front move 90° clockwise
print(c.left) #print the left side as a list (mainly for other python code to use)
\$\endgroup\$
  • 1
    \$\begingroup\$ Could you maybe add an example of how you'd use it? It would help to make a better review to understand every parameters, for example. \$\endgroup\$ – IEatBagels Nov 29 '18 at 13:38
1
\$\begingroup\$

Cube class

I feel like your form parameter would be something taken out of a command line argument. With possible values like 32, 32p, 3, etc.

But if I'm using your code, I don't know what's the use of the form parameter, it's actually very confusing to see that its a string parameter. You should probably have two parameters, patterned and size. This would be much more intuitive.

OOP wise, I don't think the save and load methods should be part of your Cube class. Especially the load one. How do I use it? Should I initialize a "dummy" cube to then call load on it? It doesn't make much sense. If I were you, I'd get another object that would take care of the saving and loading of a cube.

Cublet class

There's a lot of very similar code in the rotate method, I think some of it (especially the if and everything below) could be extracted in another method that you could reuse in your 3 loops.

Coding style

This is pretty confusing. I had to read about three times to make sure the assignations on the first line were matching (front with front, etc) and that the changes on the second line made sense.

front, left, back, right = self.front, self.left, self.back, self.right
self.front, self.left, self.back, self.right = right, front, left, back

I'll offer two options which in my opinion might be better :

  1. Have it all on one line self.front, self.left, self.back, self.right = self.right, self.front, self.left, self.back
  2. Split the assignments on multiple lines. This is probably the best option as it makes it clear what happens. :

    self.front = self.right
    self.left = self.front
    ...
    

Considering the second option, I'm aware you might tell yourself "My code is already long enough as it is, no need to add so many other lines" but I really think it would help on readability, which would be a big plus.

If one-liners

Once again I guess you did this to "save space" but really the readability is hindered by the fact that in some places you have if statement on one line vs more than one.

if cubelet.front == None: parts.extend(("None", "None"))

vs.

if type(self.left) == tuple:
    self.left = self.left[0], sides[(sides.index(self.left[1])+1)%4]

Truth is I believe all your if statement should be on at least two lines (like the second option). It's simply easier to read (this might just be an opinion)

Spacing

Your code is very cluttered and blank lines don't cost a thing :) At least put them between your methods, it will help tremendously to read and to quickly find your way around the code.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the comments, the reason form is like this is because in the future I plan to add the ability to put something like '2x2x4p' into form for a tower cube so I decided it would be best for it to be flexible \$\endgroup\$ – 13ros27 Nov 29 '18 at 15:49
  • \$\begingroup\$ You could use a tuple kind of like numpy uses for its arrays initialization. It's something like np.array(dim=(2,2,4)). \$\endgroup\$ – IEatBagels Nov 29 '18 at 16:30
  • \$\begingroup\$ However, I also might in the future add tetrahedron puzzles which would be created like t3p or something so I am trying to create my options open \$\endgroup\$ – 13ros27 Nov 29 '18 at 16:34

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.