Skip to content

duchuy-new - #6

Open
nguyenduchuy wants to merge 2 commits into
masterfrom
duchuy-new
Open

duchuy-new#6
nguyenduchuy wants to merge 2 commits into
masterfrom
duchuy-new

Conversation

@nguyenduchuy

Copy link
Copy Markdown
Collaborator

No description provided.

@nguyenduchuy nguyenduchuy self-assigned this Apr 18, 2017
@UkrainianEngineer

Copy link
Copy Markdown
Owner

Destination branch should be changed from master to release

Comment thread class-Human.py

def __init__(self):
print('The super class is "Human" ')
name = ""

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's much better to use constructor for this purpose.

Comment thread class-Human.py

class Student(Human):

marks = [] #list of student's marks

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comments before line you want to comment.

Also, please use full sentences:

"List of `Student`'s marks."

Comment thread class-Human.py

def __init__(self):
print('Object of class "Student" is created')
super().__init__()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't make any sense.
Please use a super method properly.

Comment thread class-Human.py
def get_average_mark(self):
sum = 0
for mark in self.marks:
sum += mark

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong variable name.

It looks much easier:
return sum(self.marks)/float(len(self.marks))

@UkrainianEngineer

Copy link
Copy Markdown
Owner

@nguyenduchuy it would be great to do something with this pull request.

Possible cases are:

  1. Close pull request.
  2. Fix all comments and prepare this pull request for merging into release-2 branch.

As you wish.

@Sasha1152 Sasha1152 added invalid and removed invalid labels Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants