4
\$\begingroup\$

I just deep dived to Python and I came with this Hackerrank problem:

Create two class, Person with firstName, lastName, idNumber as its attributes with method printPerson() simply print last name and first name and id, and the derived class Student.

The Student class has additional attributes, its score and an array with two value, and I need to calculate the average of all scores in this array. Then finally I will decide which letter I should assign to this average score and print it.

Sample Input:

Heraldo Memelli 8135627
2
100 80

Sample Output:

Name: Memelli, Heraldo
ID: 8135627
Grade: O

My code in Python 3:

class Person:
    def __init__(self, firstName, lastName, idNumber):
        self.firstName = firstName
        self.lastName = lastName
        self.idNumber = idNumber
    def printPerson(self):
        print("Name:", self.lastName + ",", self.firstName)
        print("ID:", self.idNumber)

def avg(a):
    return sum(a)/len(a)

def m(a):
    return [int(r) for r in list(a.values())[0].split('-')]

class Student(Person):
    def __init__(self, f, l, i, s):
        Person.__init__(self, f, l, i)
        self.s = s
    def calculate(self):
        avgScore = avg(self.s) 
        scoreList = [
          {'O': '90-100'},
          {'E': '80-89'},
          {'A': '70-79'},
          {'P': '55-69'},
          {'D': '40-54'},
          {'T': '0-39'},
        ]
        scoreLetter = list([x for x in scoreList if m(x)[0] <= avgScore == avgScore <=m(x)[1]][0].keys())[0]
        return '{}'.format(scoreLetter)

line = input().split()
firstName = line[0]
lastName = line[1]
idNum = line[2]
numScores = int(input()) # not needed for Python
scores = list( map(int, input().split()) )
s = Student(firstName, lastName, idNum, scores)
s.printPerson()
print("Grade:", s.calculate())

It passes all test cases. Could you please help me to review it and give me some advice?

\$\endgroup\$
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – Toby Speight Jan 22 at 11:25
5
\$\begingroup\$
  • Read PEP8 the Python style guide

    Functions and variables should be lower_snake_case

  • Naming

    Single letter variable names are a bad habit. Code should be self explanatory and function names like m() don't tell what they are supposed to do

  • Instead of the printPerson() function, override the magic function __str__

    Doing this you can do print(student) to print the class

  • You should make the avg function a part of the Student class

  • Your grade function can be simplified

     scoreList = [
       {'O': '90-100'},
       {'E': '80-89'},
       {'A': '70-79'},
       {'P': '55-69'},
       {'D': '40-54'},
       {'T': '0-39'},
    ]
    

    When you swap the key and values around, and loop over them in order

    self.score_dictionary = {
        90 : 'O',
        80 : 'E',
        70 : 'A',
        55 : 'P',
        40 : 'D',
        0  : 'T'
    }
    

    It becomes clear that if the average is higher than the value that should be the grade to give, else go to the next value

    This will happen automatically with Python3.7+, or else you can make use of an OrderedDict

    from collections import OrderedDict
    self.score_dictionary = OrderedDict([
        (90, 'O'),
        (80, 'E'),
        (70, 'A'),
        (55, 'P'),
        (40, 'D'),
        (0, 'T')
    ])
    

    Thnx @Graipher, for correcting my mistake regarding the OrderedDict

    Another way (which might be clearer) is to loop over the dictionary keys in sorted order

  • You can unpack multiple arguments in one go

    line = input().split()
    firstName = line[0]
    lastName = line[1]
    idNum = line[2]
    

    Could be

    f, l, i = input().split()
    
  • It is Python idion to use _ for variables you don't use

Code

class Person:
    def __init__(self, first_name, last_name, id):
        self.first_name = first_name
        self.last_name = last_name
        self.id = id

    def __str__(self):
        return f"Name: {self.last_name}, {self.first_name}\nID: {self.id}"

class Student(Person):
    def __init__(self,  first_name, last_name, id, scores):
        super().__init__(first_name, last_name, id)
        self.scores = scores
        self.avg = sum(self.scores) / len(scores)
        self.score_dictionary = {
            90 : 'O',
            80 : 'E',
            70 : 'A',
            55 : 'P',
            40 : 'D',
            0  : 'T'
        }

    def grade(self):
        for score in sorted(self.score_dictionary.keys(), reverse=True):
            if self.avg >= score:
                return self.score_dictionary[score]

if __name__ == '__main__':
    f, l, i = input().split()
    _ = input()
    scores = list(map(int, input().split()))
    student = Student(f, l, i, scores)
    print(student)
    print(f"Grade: {student.grade()}")
\$\endgroup\$
  • \$\begingroup\$ And the naming scheme and having to define a printPerson method was required in the problem, but recommending the normal best-practice is still good. \$\endgroup\$ – Graipher Jan 22 at 9:19
  • \$\begingroup\$ Thank you @Graipher and Ludisposed, I upvoted all of you, I will try to understand, thank you again for helping me! \$\endgroup\$ – chau giang Jan 22 at 10:32

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.