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?

David Robinson
  • 77,383
  • 16
  • 167
  • 187
JamieB
  • 923
  • 9
  • 30
  • 2
    This is a nearly perfect question for http://codereview.stackexchange.com/. (ETA: see the [Code Review FAQ](http://codereview.stackexchange.com/faq) for how it differs from StackOverflow- typically asking how to improve your *working* code code belongs on the former) – David Robinson Apr 22 '12 at 16:42
  • Is it ok to stay here for the moment? I might need to ask some functional questions. – JamieB Apr 22 '12 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) – David Robinson Apr 22 '12 at 16:46

2 Answers2

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
Community
  • 1
  • 1
tbraun89
  • 2,246
  • 3
  • 25
  • 44
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.

Ohad
  • 2,752
  • 17
  • 15