3
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 "<"+ str(self.x) + "," + str(self.y) + ">"


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

That is the code I have so far, Shape is supposed to represent an abstract class and the other two classes are supposed to be inheriting from it, to me it still looks too hard coded to be a abstract solution, how could I improve?

3
  • 2
    This is a nearly perfect question for codereview.stackexchange.com. (ETA: see the Code Review FAQ for how it differs from StackOverflow- typically asking how to improve your working code code belongs on the former) Commented Apr 22, 2012 at 16:42
  • Is it ok to stay here for the moment? I might need to ask some functional questions. Commented Apr 22, 2012 at 16:45
  • Functional questions about this specific code would be better asked and more likely to be answered on Code Review. And if you have new questions aside from "How can I make this code better", they should probably be separate questions on StackOverflow. (Again, nothing is wrong with your question, I'm just proposing a site you could get more helpful solutions) Commented Apr 22, 2012 at 16:46

2 Answers 2

4

I would change this part in the abstract class:

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

You could specify a default in the abstract class and override the method in Rectangle or in Circle.

But you may get a better answer on https://codereview.stackexchange.com/.

UPDATE (Example):

from abc import ABCMeta, abstractmethod

class Shape: 
  __metaclass__ = ABCMeta

  @abstractmethod
  def getArea(self):
    pass

class Rectangle (Shape):
  def getArea(self):
    return self.width * self.height

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

UPDATE 2 (Overloading functions)

Like Ohad wrote, overloading doesn't work in python here is an example without overloading the init funcktion:

def Shape:
  def __init__(self, centrePoint, colour, **kwargs):
    self.centrePoint = centrePoint
    self.colour      = colour
    self.width       = kwargs.get('width')
    self.height      = kwargs.get('height')
    self.radius      = kwargs.get('radius')

Now you can create objects with

rect = Rectangle(0, "red", width=100, height=20)
circ = Circle(0, "blue", radius=5)

A good post about kwargs is here: What is a clean, pythonic way to have multiple constructors in Python?

The type variable is also useless because you can use this to identify the type:

>>> rect = Rectangle(...)
>>> print isinstance(rect, Rectangle)
True
>>> print isinstance(rect, Circle)
False
Sign up to request clarification or add additional context in comments.

1 Comment

+1 But you could also give an example of what to replace it with.
2

First, note that there is no function overloading in python, thus this:

class Shape:        
    def __init__(self, centrePoint, colour, width, height):
        ...

    def __init__(self, centrePoint, radius, colour):
        ...

will get your first function 'overridden' by the second.

As a general rule of thumb, the main idea behind the concept of a common Base/Abstract class is to decouple the implementation from the interface (note the python conventions, lower_case_with_underscores for function names, not camleCase):

class Shape: 
  __metaclass__ = ABCMeta

  @abstractmethod
  def get_area(self):
    pass

  move, scale, scale_vertically, scale_horizontally, print...

Now that I have my abstract yet operable (as in 'Every shape can be scaled, moved..') base class, which is completely unaware of how a circle is constructed, moved or scaled. I can then start to implement the sub-classes: Rectangle, Circle, and so on.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.