Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Week3 #1

Open
wants to merge 17 commits into
base: week3
Choose a base branch
from
Open

Week3 #1

wants to merge 17 commits into from

Conversation

JustinQ-DNHS
Copy link

probably did something right

@NoahSimon8
Copy link

Nice work, and great job getting this in just one day. Few things though

  1. Your attack variable should be private, same as the rest of your instance variables here. NIce job with the additional implementation, but gotta stick with encapsulation. To make it work in the child class, because you need a reference, you might want to make a getter method. Additionally, when referencing the parent class in the child, please use super instead of this. Both work (in this case at least), but super is better practice and organizationally more clear

  2. When running, did you always get the output you expected? The condition in the while loop seems a little suspect; it will loop "while true", and you have the conidition as "if jigglypuff or pickachu have fainted". That should be false at the start, so the loop is not actually running. You get a print statement anyways just becasue you have an "else" statement

  3. Please give a little more to your explanations, much of what you wrote was essentially in the instructions. For example, please explain a time you used method polymorphism, not just say that you did. That's what part of the code is the polymorphism, what its accomplishing, why it's useful

  4. Your explanation of subtype polymorphism is referencing a part of the code that does not exemplify subtype polymorphism

Depending on what you think you need, actually fix these things or dont. If you do, lmk

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.

2 participants