Implementing a rectangle class

Question

First I've to indicate that this is my homework.

An example of a given input is:

s1 = {'p1': (x1, y1), 'p2': (x2, y2)}

where all names and coordinations are user input, only the input format is predefined; also p1 is the upper leftmost point and p2 is the lower rightmost point as shown below:

I can go on to get area, surrounding, midpoint, height and width based on this input.

I wrote a code that works as demanded but I don't like it. it doesn't seem I followed the Pythonic approach here; how can I improve this?

class rectangle:
    def __init__(self, dct):
        # ['name', [x, y]]
        names = list(dct.keys())
        coords = list(dct.values())
        start = [names[0], coords[0]]
        end = [names[1], coords[1]]
        self.start = start
        self.end = end

        # int
        start_coords = self.start[1]
        end_coords = self.end[1]
        width = end_coords[0] - start_coords[0]
        height = end_coords[1] - start_coords[1]
        self.width = width
        self.height = height

        # [x, y]
        midpoint = [self.width / 2, self.height / 2]
        self.midpoint = midpoint

        # int
        area = (self.width + self.height) * 2
        surr = self.width * self.height
        self.area = area
        self.surr = surr

s1 = rectangle({'p1': (1,1), 'p2': (2,2)})
print('point one: ', s1.start)
print('point two: ', s1.end)
print('width: ', s1.width)
print('height: ', s1.height)
print('midpoint: ', s1.midpoint)
print('area: ', s1.area)
print('surrounding: ', s1.surr)

Answer 1

PEP-8

• Class names should be CapWords, so instead of rectangle you should have Rectangle.
• Commas should be followed by 1 space. You've mostly followed this, except in s1 = rectangle({'p1': (1,1), 'p2': (2,2)})

Bugs

• The formula for "area" is not twice the sum of width & height.
• I don't know what "surrounding" is, but the formula for perimeter is not width times height.
• The midpoint (centre?) of a rectangle should be within the bounds of the rectangle. Consider the rectangles with corners (10, 10) and (12, 12). The centre would be (11, 11), not (1, 1) as calculated.

Awkward initialization

This code:

        names = list(dct.keys())
        coords = list(dct.values())
        start = [names[0], coords[0]]
        end = [names[1], coords[1]]
        self.start = start
        self.end = end

relies on the dictionary's ordering of keys. It can break in Python 3.6 and earlier (CPython 3.5 and earlier). It does not enforce the key names p1 and p2; any two keys will work. And self.start[0] and self.end[0] are never used, so storing the key names in these entries is unnecessary.

The code could simply and safely read:

         self.start = dct['p1']
         self.end = dct['p2']

with suitable modifications of the usage of self.start and self.end.

Class with no methods

A class should have methods. Without any methods, you'd be better off with a namedtuple for constant data, or a dict for mutable data.

So let's give your class some methods:

    def width(self):
        return self.end[0] - self.start[0]

    def height(self):
        return self.end[1] - self.start[1]

As mentioned by Peilonrayz, you may wish to use abs(...) here.

You can use these methods externally:

print('width: ', s1.width())
print('height: ', s1.height())

as well as in other members of this class:

    def area(self):
        return (self.width() + self.height()) * 2    # Note: Formula is still incorrect

An Over-Engineered Solution

Do not submit this as your home-work solution! You would likely fail or be expelled! This illustrates some advanced concepts like the @dataclass and the NamedTuple, type hints and the typing module, as well as read-only @property attributes, a @classmethod, and """docstrings""". You may find these interesting to study in your free time.

from typing import NamedTuple
from dataclasses import dataclass

class Point(NamedTuple):
    x: float
    y: float

@dataclass
class Rectangle:
    """A class for calculations on a Rectangle"""

    p1: Point
    p2: Point

    @classmethod
    def from_dict(cls, dct):
        """
        Constructs a Rectangle from a dictionary with "p1" and "p2" keys.
        These keys must contain a tuple or list of two numeric values.
        """
        return Rectangle(Point._make(dct['p1']), Point._make(dct['p2']))

    @property
    def width(self):
        """
        Computes the width of the rectangle.
        """
        return abs(self.p2.x - self.p1.x)

    @property
    def height(self):
        """
        Computes the height of the rectangle.
        """
        return abs(self.p2.y - self.p1.y)

    @property
    def area(self):
        """
        Incorrectly computes the area of the rectangle.
        """
        return (self.width + self.height) * 2     # Note: still the incorrect formula

s1 = Rectangle.from_dict({'p1': (1,1), 'p2': (2,2)})
print('point one: ', s1.p1)
print('point two: ', s1.p2)
print('width: ', s1.width)
print('height: ', s1.height)
print('area: ', s1.area)

Output:

point one:  Point(x=1, y=1)
point two:  Point(x=2, y=2)
width:  1
height:  1
area:  4

Answer 2

• Class should begin with capital, so Rectangle
• You are creating local variables just to set them to object on next line using self, assign calculation to self.xyz variable directly. For example:

        self.area = (self.width + self.height) * 2
        self.surr = self.width * self.height

• Creating list of keys, then list of values to then map it to different structure seems very obscure. Take a look at items. I think you can change your code to:

        items = dct.items()
        start = items[0]
        end = items[1]

Still, I don't understand, why are you doing this. I'd just access those values directly from original dct, I find it more readable and clean, ex:

width = dct["p2"][0] - dct["p1"][0]

Answer 3

Following the advice from K.H. we get:

class Rectangle:
    def __init__(self, dct):
        items = dict.items()
        self.start = items[0]
        self.end = items[1]

        self.width = self.end[1][0] - self.start[1][0]
        self.height = self.end[1][1] - self.start[1][1]
        self.midpoint = [self.width / 2, self.height / 2]
        self.area = (self.width + self.height) * 2
        self.surr = self.width * self.height

This still has a couple of problems:

• By passing dct as a dict you have made two assumptions:

  1. Dictionaries are ordered by default now but on 3.5 and before they are not.
  2. A user will always enter the start as the first value of the dictionary.

  These are bad because you've made some assumptions without being explicit. To solve this you can just pass start and end to Rectangle.

• Start is assumed to have lower values than end. This means self.width and self.height can be negative values, if this assumption no longer holds. A negative width or height don't make much sense.

  This assumption also goes on to effects self.area and self.surr.

• Start and end don't make too much sense to return a key, that isn't ever used in Rectangle.

• Area and surr are wrong.

  • The equation for area is \$ab\$ not \$2(a + b)\$.
  • The equation for surface area, perimeter, is \$2(a + b)\$ not \$ab\$.

class Rectangle:
    def __init__(self, start, end):
        self.start = start
        self.end = end

        self.width = abs(self.end[0] - self.start[0])
        self.height = abs(self.end[1] - self.start[1])
        self.midpoint = [self.width / 2, self.height / 2]
        self.area = self.width * self.height
        self.surr = (self.width + self.height) * 2