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

Dylan Reid - Pet Park Assignment #37

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

Conversation

dylanreid7
Copy link

No description provided.


// define struct of animal type, age, and gender
struct animalAndDemo {
uint animalType;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use enums for animal type and gender

address owner;

// define struct of animal type, age, and gender
struct animalAndDemo {
Copy link
Contributor

Choose a reason for hiding this comment

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

The general convention is to have the first letter as uppercase in names of structs, classes and events

emit Added(animalType, count);
}

function borrow(uint age, uint gender, uint animalType) public animalRange(animalType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be marked external since it is not being called anywhere internally


function add(uint animalType, uint count) public animalRange(animalType) {
// check that only owner has access
require(owner == msg.sender, "Not an owner");
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to check this with a modifier as it make the code more readable


function animalCounts(uint animalType) public view returns(uint) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since solidity public mappings are automatically exposed as functions, you can just make animalCounts as a public mapping. Here you just have to rename animalsInZoo to animalCounts


// set animal type back to zero in borrow mapping
uint currentAge = borrowedAnimal[msg.sender].age;
uint currentGender = borrowedAnimal[msg.sender].gender;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since borrowedAnimal[msg.sender] is being used multiple times it can be stored in a single memory variable. Later in the course you will learn that it is a gas optimisation technique

uint gender;
}

// animals in zoo: animalType and count of that animal in the zoo
Copy link
Contributor

Choose a reason for hiding this comment

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

In solidity it is recommended to use Natspec format for commenting

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