1
\$\begingroup\$

I used Pythons source code/documentation/textbook to come up with an abstract class and following subclasses.

My requirements are:

  • Domestic students don't require documentation
  • International student do require documentation (passports, etc..)

    from abc import ABCMeta
    
    class Student(metaclass=ABCMeta):
    
        def __init__(self, id, firstname, lastname):
            self.__id = id
            self.__firstname = firstname
            self.__lastname = lastname
    
        @property
        def iden(self):
            return self.__id
    
        @property
        def first_name(self):
            return self.__firstname
    
        @property
        def last_name(self):
            return self.__lastname
    

International Subclass:

from Student import Student
from copy import deepcopy


class International(Student):
    def __init__(self, iden, firstname, lastname, docuemnts):
        super().__init__(iden, firstname, lastname)

        self.__documents = deepcopy(docuemnts)
        self.__type_of_student = "International"

    @property
    def international_documents(self):
        return deepcopy(self.__documents)

Domestic Subclass:

from Student import Student


class Domestic(Student):

    def __init__(self, iden, firstname, lastname):
        super().__init__(iden, firstname, lastname)

        self.__type_of_student = "Domestic"

    @property
    def student_type(self):
        return self.__type_of_student

Have I constructed and implementation the abstract class correctly?

\$\endgroup\$
1
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Peilonrayz
    Oct 31, 2017 at 13:03

1 Answer 1

1
\$\begingroup\$
  • How do you tell what the student type is, without erroring on both International and Domestic?

    With your current code, you'd have to do something like:

    if hasattr(student, 'student_type'):
        student_type = student.student_type()
    else:
        student_type = None
    

    What makes this worse, is you assume None to be International.

  • Why can't Domestic students have international_documents?

    In your requirements you state:

    • Domestic students don't require documentation
    • International student do require documentation (passports, etc..)
  • Why should these be separate classes?

  • Your name mangled variables make your code longer, and generally more horrible to read. Instead you could make them just normal properties.

    However, since they are unsetable, you may want to use a namedtuple instead.

And so I'd use something like:

from collections import namedtuple
from Enum import enum

class StudentType(Enum):
    INTERNATIONAL = 'international'
    DOMESTIC = 'domestic'

Student = namedtuple('Student', 'id first_name last_name type documents')

s1 = Student(0, 'John', 'Doe', StudentType.INTERNATIONAL, [...])
s2 = Student(1, 'Eva', 'Smith', StudentType.DOMESTIC, [])
\$\endgroup\$
3
  • \$\begingroup\$ Reading your comments made me realize I screwed up when I posted this code. The abstract class is supposed to contain StudentType, allow me to correct this. \$\endgroup\$
    – user150904
    Oct 31, 2017 at 12:53
  • \$\begingroup\$ Given that domestic can't have documents, I needed a way for client not to pass documents. I don't want to trust the client code too much, if I used to namedtuple, there is nothing stopping client code from pass documents to domestic. \$\endgroup\$
    – user150904
    Oct 31, 2017 at 12:59
  • \$\begingroup\$ @S.R. Yes, however what if a domestic needs documents linked to their account? Also your code doesn't make it so that internationals need documents, they can pass an empty list of documents. You can also always subclass Student so that it's created with the validation you need. \$\endgroup\$
    – Peilonrayz
    Oct 31, 2017 at 13:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.