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.

Sign up for GitHub

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

Jump to bottom

Della - Pet Park Assignment #19

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Della - Pet Park Assignment #19

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
70ca263
add contract, 9/14 tests passing
blackqueentech May 16, 2022
ceef751
add in logic for women cat for testing
blackqueentech May 17, 2022
dbbfe1b
11/14 passing, confused on ordering of logic
blackqueentech May 19, 2022
1597a1d
13/14 passing, zero value gender logic unclear
blackqueentech May 19, 2022
eeaaf60
13/14, female cat test issue
blackqueentech May 23, 2022
979503d
13/14, female 40 issues still
blackqueentech May 23, 2022
19a1686
13/14, invalid gender for female cat case
blackqueentech May 23, 2022
b3ef630
13/14 new if block, female test failing
blackqueentech May 24, 2022
87d2526
passing tests
blackqueentech May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions contracts/PetPark.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ contract PetPark {
revert("Invalid Age");
}

if (u.gender != _gender) {
if (
u.gender != _gender ||
(u.gender == Gender.Female && _gender == Gender.Male) ||
(u.gender == Gender.Male && _gender == Gender.Female)
) {
Copy link
Author

Choose a reason for hiding this comment

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

I'm still not able to pass the female under 40 borrowing cats because it hits this test. Here is my logic for the code:

u.gender != _gender - this is the clear incorrect case.
(u.gender == Gender.Female && _gender == Gender.Male) - this is an example of another clear case, feels redundant but it shows genders not matching
(u.gender == Gender.Male && _gender == Gender.Female) - this is the case I have an issue with. If I am understanding solidity correctly, before an address is added to the map, it will default to male since it is first in the Gender enum. Therefore, if an address calls borrow for the first time and is of Gender.Female, this will always fail because it won't match the default option on the enum. I need clarification as to how to get around this. I don't know why there isn't a need for a Gender.None enum option to avoid this, like there is for AnimalType.

revert("Invalid Gender");
}

Expand All @@ -93,8 +97,8 @@ contract PetPark {

require(u.animal == AnimalType.None, "Already adopted a pet");

if (u.gender == Gender.Female) {
if (_age < 40 && _animal == AnimalType.Cat) {
if (u.gender == Gender.Female && u.age < 40) {
if (_animal == AnimalType.Cat) {
revert("Invalid animal for women under 40");
}
}
Expand Down