HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Abstractness of Shape class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
abstractnessclassshape

Problem

Shape represents an abstract class and the other two classes inherits from it. This looks insufficiently abstract to me. How could I improve it?

import math
class Point:

    def __init__(self,x,y):
        self.x = x
        self.y = y

    def move(self,x,y):
        self.x += x
        self.y += y

    def __str__(self):
        return ""

class Shape:        
    def __init__(self, centrePoint, colour, width, height):
        self.centrePoint = centrePoint
        self.colour = colour
        self.width = width
        self.height = height
        self.type = "Square"

    def __init__(self, centrePoint, radius, colour):
        self.type = "Circle"
        self.radius = radius
        self.colour = colour
        self.centrePoint = centrePoint

    def move(self,x,y):
        self.centrePoint.move(x,y)

    def getArea(self):
        if (self.type == "Square"):
            return self.width * self.height
        elif (self.type == "Circle"):
            return math.pi*(self.radius**2)

    def __str__(self):
        return "Center Point: " + str(self.centrePoint) + "\nColour: "+ self.Colour + "\nType: " + self.type + "\nArea: " + self.getArea()

class Rectangle (Shape):

    def scale(self, factor):
        self.scaleVertically(factor)
        self.scaleHorizontally(factor)

    def scaleVertically(self, factor):
        self.height *= factor

    def scaleHorizontally(self, factor):
        self.width *= factor

class Circle (Shape):

    def scale(self, factor):
        self.radius * factor

Solution

Your class Shape knows about what kind of shape it is (circle/square). This makes it tied to the two child classes. It should not know any thing about the composition. Rather just provide methods with empty implementations that can be overridden by child classes.

The only thing I can see that is common across all classes is the concept of a center point and a color. So

import abc

class Shape(metaclass=abc.ABCMeta):        
    def __init__(self, centrePoint, colour):
        self.centrePoint = centrePoint
        self.colour = colour


Since move depends only on center point, it is ok to define it in base class.

def move(self,x,y):
        self.centrePoint.move(x,y)


This is certainly one that should be implemented by the child classes because it requires knowledge about what kind of a shape it is.

@abc.abstractmethod
    def getArea(self):
        return


or if you dont want to use it

def getArea(self):
        raise NotImplementedError('Shape.getArea is not overridden')


see
abc in
so

And the to string method is another that should be done in child because it holds details of what shape it is.

@abc.abstractmethod
    def __str__(self):
        return


However, if you are going to use the same format for all shapes, then you can provide the default implementation (as below.)

def __str__(self):
        return "Center Point: " + str(self.centrePoint) + "\nColour: "+ self.Colour + "\nType: " + self.type() + "\nArea: " + self.getArea()


I would also recommend scale as part of the base class because all shapes can be scaled.

def scale(self, factor):
        return


Basically all operations that make sense on any shapes should have an abstract method on the base class.

Now for child classes

class Rectangle (Shape):
    def __init__(self, cp, color, width, height):
        super(Rectangle, self).__init__(cp, color)
        self.width = width
        self.height = height

    def scale(self, factor):
        self.scaleVertically(factor)
        self.scaleHorizontally(factor)

    def scaleVertically(self, factor):
        self.height *= factor

    def scaleHorizontally(self, factor):
        self.width *= factor

    def getArea(self):
        return self.width * self.height

    def type(self):
        return "Rectangle"

class Circle (Shape):
    def __init__(self, cp, color, radius):
        super(Rectangle, self).__init__(cp, color)
        self.radius = radius

    def scale(self, factor):
        self.radius * factor

    def getArea(self):
        return math.pi*(self.radius**2)

    def type(self):
        return "Circle"

Code Snippets

import abc

class Shape(metaclass=abc.ABCMeta):        
    def __init__(self, centrePoint, colour):
        self.centrePoint = centrePoint
        self.colour = colour
def move(self,x,y):
        self.centrePoint.move(x,y)
@abc.abstractmethod
    def getArea(self):
        return
def getArea(self):
        raise NotImplementedError('Shape.getArea is not overridden')
@abc.abstractmethod
    def __str__(self):
        return

Context

StackExchange Code Review Q#11080, answer score: 2

Revisions (0)

No revisions yet.