3

I'm doing some OOP in Python and I'm having trouble when the user inputs a tuple as one of the arguments. Here's the code:

class Height:
    def __init__(self,ft,inch=0):
        if isinstance(ft,tuple):
            self.feet = ft
            self.inches = inch
        elif isinstance(ft,int):
            self.feet = ft // 12
            self.inches = ft % 12
    def __str__(self):
        return str(self.feet) + " Feet " + str(self.inches) + " Inches"
    def __repr__(self):
        return "Height (" + str(self.feet * 12 + self.inches) + ")"

I tried thought initializing the inch to 0 would help thing out but that didn't work. Tuples also don't support indexing so that option was also non-existent. I feel like the answer is simple and I'm just overthinking it. The test code that I'm using is:

from height import *
def test(ht):
    """tests the __str__, __repr__, and to_feet methods for the height
   Height->None"""
    #print("In inches: " + str(ht.to_inches()))
    #print("In feet and inches: " + str(ht.to_feet_and_inches()))
    print("Convert to string: " + str(ht))
    print("Internal representation: " + repr(ht))
    print()
print("Creating ht1: Height(5,6)...")
ht1 = Height(5,6)
test(ht1)
print("Creating ht2: Height(4,13)...")
ht2 = Height(4,13)
test(ht2)
print("Creating ht3: Height(50)...")
ht3 = Height(50)
test(ht3)

My code works as expected when an int is input but, again, I can't seem to figure it out when a tuple is input. Any ideas?

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
  • 4
    tuples do support indexing. – Padraic Cunningham May 03 '15 at 20:31
  • Having a star trek reference in the question title gets an upvote from me :) – krock May 03 '15 at 20:36
  • 1
    @PadraicCunningham (Since when does `a = (2, 0); print(a[0])` throw an IndexError ? Tuples support indexing...) Sorry, forget what I've said, I misread your comment... My apologizes!! – Spirine May 03 '15 at 20:36
  • 1
    Why do you *want* a tuple input? Why would the user call `Height((5, 5))` instead of `Height(5, 5)`? Note you can unpack a tuple as arguments: `Height(*(5, 5))` (see e.g. http://stackoverflow.com/q/36901/3001761). Also, you seem to be mixing up *feet* and *inches*, which is a pretty critical failure when that's the core of your code. – jonrsharpe May 03 '15 at 20:37
  • Nowhere in your test code are you passing in a tuple... just two arguments. – seaotternerd May 03 '15 at 20:38

3 Answers3

4

Are you really being passed a tuple? It looks to me that your constructor should simply be:

def __init__(self, ft, inch=0):
   self.ft = int(ft)
   self.inch = int(inch)

That works if you create your object with any of these (because you have a default value for the inch argument):

foo = Height(6)
bar = Height(6, 3)
baz = Height("5", 3)

etc. Note that in the second and third instances, you still aren't being passed a tuple. In order to actually receive a tuple, you'd need to call it like this:

foo2 = Height((6, 3))

or use the '*' operator in the constructor declaration.

Gil Hamilton
  • 11,973
  • 28
  • 51
  • While this is the better way to go about things, I should point out that Trevor's Height object takes interprets `ft` as being in inches if there is only one argument. – cge May 03 '15 at 20:46
  • Good point. Actually makes more sense to maintain the value internally in one unit anyway (either feet or inches [or mm or whatever]), then convert it to human-familiar feet/inches on output, as needed. – Gil Hamilton May 03 '15 at 20:50
1

Tuples do support indexing. They don't support index assignment, because they are immutable, but do support index access.

Your code isn't working because your code is designed to accept the tuple in as ft. It isn't doing anything to int, which is an entirely different argument. All you need to do is use indexing instead:

class Height:
    def __init__(self,ft,inch=0):
        if isinstance(ft,tuple):
            self.feet = ft[0]
            self.inches = ft[1]
        elif isinstance(ft,int):
            self.feet = ft // 12
            self.inches = ft % 12
    def __str__(self):
        return str(self.feet) + " Feet " + str(self.inches) + " Inches"
    def __repr__(self):
        return "Height (" + str(self.feet * 12 + self.inches) + ")"

However, this only works if you actually pass in a tuple, which your test code never does! Height(5,6) is just passing in two arguments. For a tuple input, you'd need Height((5,6)).

As others have noted, the way you are calling it actually makes more sense than using a tuple. To get that working, you just need to see if your second argument is being used (and let's change to v1 and v2, for reasons I'll get to later):

def __init__(self, v1, v2=None): # set v2 to None, and then check to see if it isn't None
    if v2 is not None:
        self.feet = v1
        self.inches = v2
    else:
        self.feet = v1 // 12
        self.inches = v1 % 12

But this has a usability problem: the meaning of the first argument depends on whether there is a second argument there! That's why naming it ft is particularly bad: if the second argument isn't provided, it's actually in inches!

One solution to this is to make both arguments have names, allowing the user to choose the ones they want to use:

def __init__(self, feet=0, inches=0):
    self.feet = feet + inches // 12
    self.inches = inches % 12

Then you have many choices, and less chance for confusion:

In [4]: str(Height(5,0))
Out[4]: '5 Feet 0 Inches'

In [5]: str(Height(0,5))
Out[5]: '0 Feet 5 Inches'

In [6]: str(Height(5))
Out[6]: '5 Feet 0 Inches'

In [7]: str(Height(inches=25))
Out[7]: '2 Feet 1 Inches'

In [8]: str(Height(feet=3,inches=25))
Out[8]: '5 Feet 1 Inches'
cge
  • 9,552
  • 3
  • 32
  • 51
1

You have numerous problems in your code, from formatting to functionality. Most important is this:

elif isinstance(ft,int):
    self.feet = ft // 12
    self.inches = ft % 12

so if the user passes a number in feet, it's assumed to actually be in inches? Why?!


I think you are fundamentally mistaking what happens when you call e.g.

Height(5, 10)

Inside __init__, this effectively sets ft = 5 and inch = 10, not ft = (5, 10).


Here's how I'd have done it:

class Height:

    def __init__(self, feet=0, inches=0):
        self.feet, self.inches = feet, inches % 12
        self.feet += inches // 12

    def __str__(self):
        return '{0.feet}\' {0.inches}"'.format(self)

    def __repr__(self):
        return 'Height({0.feet}, {0.inches})'.format(self)

This allows any of the following to work correctly:

>>> Height()
Height(0, 0)
>>> Height(5, 0)
Height(5, 0)
>>> Height(5)
Height(5, 0)
>>> Height(inches=60)
Height(5, 0)
>>> print(str(Height(inches=123)))
10' 3"

Note also use of str.format and compliance with the style guide. Another alternative would be to raise an error if inches > 11 and feet > 0, as that is potentially erroneous input. You might also want to look into emulating numeric types.

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437