patternpythonMinor
Abstractness of Shape class
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 * factorSolution
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
Since move depends only on center point, it is ok to define it in base class.
This is certainly one that should be implemented by the child classes because it requires knowledge about what kind of a shape it is.
or if you dont want to use it
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.
However, if you are going to use the same format for all shapes, then you can provide the default implementation (as below.)
I would also recommend scale as part of the base class because all shapes can be scaled.
Basically all operations that make sense on any shapes should have an abstract method on the base class.
Now for 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 = colourSince 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):
returnor 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):
returnHowever, 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):
returnBasically 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 = colourdef move(self,x,y):
self.centrePoint.move(x,y)@abc.abstractmethod
def getArea(self):
returndef getArea(self):
raise NotImplementedError('Shape.getArea is not overridden')@abc.abstractmethod
def __str__(self):
returnContext
StackExchange Code Review Q#11080, answer score: 2
Revisions (0)
No revisions yet.