search

kyzooghost / blockchain-pharmacy

Blockchain Pharmacy
https://blockchain-developer-bootcamp-final-project-gold-nu.vercel.app
0 stars 0 forks source link

Error message handling and optimisations #1

Open thedarkjester opened 2 years ago

thedarkjester commented 2 years ago

I like that you are unable to prescribe to yourself. That shows use-case thinking. It would be good to see a simplified error vs. the entire failure response from your reverting in the contract.

The network switching was seamless and works very well - nicely done. It is well placed for network changes, but It would be good not to have a full refresh when changing between accounts. This is more a comment on single page applications and user interaction vs. a failing in the project.

One final note on interaction - the spinner showing something pending is good, I would consider using an additional note indicating that it is still waiting for confirmation. I tested navigating away and coming back - worked perfectly.

With respect to design patterns, you could have included Inheritence with the access control.

Some optimisations:

  1. SafeMath is no longer needed with compiler v 0.8.x - the overflows will revert now by default
  2. Data is viewable on chain if you dig through it, so putting the modifiers and reverts on the getters don't really hide the data
  3. When you construct your structs you could optimise gas by using default values (bool false, uints 0 etc) and just assign what you need
Script newScript;
script.prescriptionId = prescriptionId;
...
scripts.push(newScript);

One final note - great job on optimising for an end product with tests vs. implementing it all in the UI.

kyzooghost commented 2 years ago

Hey @thedarkjester really appreciate the detailed feedback 🙏