3
\$\begingroup\$

My girlfriend and I are making a goodies tracker to enable us to keep better track of our stamp/gift cards. Right now we've got a super simple Object with some fields of what a stamp card has, and no support for modifying its fields (yet). I want to get some tips on if this is bad or good code, if there are any glaring bugs, and if possible if you'd know any good ways to look at how to integrate similar Objects like gift card and stamp card (inheritance, or composition?)

There are some dependencies which you can choose if you want to download. I'll include the output of the code here:

current output

# -*- coding: utf-8 -*-
from tabulate import tabulate
# TODO: Put the dependencies into a venv so that it's easy to install
# TODO: add the amount of stamps we currently have vs. the stamp_capacity

class StampCard:
    # A stamp card for an establishment which contains a name and a capacity
    # Authors: Cody Antonio Gagnon & Chuan-Li Ojales Chang

    def __init__(self, card_name, card_description, stamp_capacity,
                 stamp_symbol, redeemable_location):
        """
        Initialize the stamp card with a:
            name
            description
            capacity
            symbol for stamping
            location to be redeemed
        """
        self.name = card_name
        self.description = card_description
        self.stamp_capacity = stamp_capacity
        self.stamp_symbol = stamp_symbol
        self.location = redeemable_location

    def get_name(self):
        """
        Return the name of the stampcard
        """
        return self.name

    def get_description(self):
        """
        Return the description of the stampcard
        """
        return self.description

    def get_capacity(self):
        """
        Return the capacity as a string.
        """
        return str(self.stamp_capacity)

    def get_symbol(self):
        """
        Return the capacity as a string.
        """
        return self.stamp_symbol

    def get_location(self):
        """
        Return the redeemable location as a string.
        """
        return self.location

    def print_stamps(self):
        """
        Print the stamps using the typical two-row approach.
        """
        stamps = []
        stamps_per_row = self.stamp_capacity / 2  # hardcoded rows for now...
        for x in range(1, 3):
            stamps.append([self.stamp_symbol] * stamps_per_row)
        print (tabulate(stamps, tablefmt="grid").encode('utf8', 'replace'))  # takes a list of lists or another tabular data type as the first argument, and outputs a nicely formatted table


def main():
    stamp = unicode("🍭", 'utf8')
    oasis = StampCard("Oasis", "Bubble Tea", 10, stamp, "University District")
    print("Name: " + oasis.get_name() + ", Capacity: " + oasis.get_capacity())
    oasis.print_stamps()

main()
\$\endgroup\$
5
\$\begingroup\$

It's weird that your get_capacity() returns a string. I'm guessing that it's to facilitate string concatenation in the print() statement in main(), but that is not a good justification for the weirdness. A better solution would be to use str.format() rather than string concatenation.

Those get_…() methods look like an awkward translation of Java code. In Python, it is often customary to just access the member variables directly. If you did want to write a getter method (to help enforce read-only access, for example), you could write a @property instead.

Going further, it seems like your StampCard is mostly a structure to hold a bunch of read-only properties. You could just use or inherit from collections.namedtuple.

# -*- coding: utf-8 -*-
from collections import namedtuple
from tabulate import tabulate

class StampCard(namedtuple('StampCard', 'name description capacity symbol redeemable_location')):
    def __str__(self):
        stamps_per_row = self.capacity // 2  # hardcoded rows for now
        stamps = [[self.symbol] * stamps_per_row] * 2
        return tabulate(stamps, tablefmt='grid')

def main():
    oasis = StampCard('Oasis', 'Bubble Tea', 10, u'🍭', 'University District')
    print('Name: {card.name}, Capacity: {card.capacity}'.format(card=oasis))
    print(unicode(oasis))

if __name__ == '__main__':
    main()
\$\endgroup\$
  • \$\begingroup\$ @MathiasEttinger I had originally implemented it based on an earlier revision of the question that claimed it was Python 3. \$\endgroup\$ – 200_success Feb 20 '18 at 8:26
  • \$\begingroup\$ Oh, ok. Didn't saw the history (by the way, it's really a hard thing to do when using the app...) \$\endgroup\$ – Mathias Ettinger Feb 20 '18 at 8:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.