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

Change CyptoPizza.sol isUnique modifier to O(1) complexity #273

Open
ernestognw opened this issue Sep 3, 2020 · 1 comment
Open

Change CyptoPizza.sol isUnique modifier to O(1) complexity #273

ernestognw opened this issue Sep 3, 2020 · 1 comment

Comments

@ernestognw
Copy link

ernestognw commented Sep 3, 2020

Summary

Crypto Pizza example project has a isUnique function that traverses the whole array of Pizzas registered. This is a O(n) behaviour that could be simplified by creating a nameToPizzaId and dnaToPizzaId mapping to achieve O(1) complexity.

Motivation

Since this is an example project for people starting to use EthereumStudio, I don't think it'll be nice to encourage this sort of searches in a modifier function. Also, I was digging in the Solidity docs to see if modifiers somehow don't consume gas, but as far as explained in this answer, they do.

Describe alternatives you've considered

We can replace the isUnique modifier content with a O(1) check to nameToPizzaId and dnaToPizzaId mappings, in order to keep function working the same but with significantly less gas consumption.

I can implement it if there's no specific reason to keep it as it is.

Additional context

The isUnique function can be seen here

And here's the solidity documentation about modifiers. This is where I couldn't find any reason why it could be accepted to have an O(n) behaviour

@shreyas-londhe
Copy link

I want to work on this

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

No branches or pull requests

2 participants