search

TheAlgorithms / Solidity

Algorithms and data structures implemented in Solidity
GNU Lesser General Public License v2.1
332 stars 95 forks source link

[fix] logic for iterative factorial function #6 #7

Closed 0xSumitBanik closed 2 years ago

0xSumitBanik commented 2 years ago

This PR provides the fix for the issue #6

Gas Comparison

Operation Execution Cost
i++ 26496 gas
++i 26456 gas
0xSumitBanik commented 2 years ago

Please feel free to review the changes. @mkubdev , @wenceslas-sanchez Thanks.

wenceslas-sanchez commented 2 years ago

This PR provides the fix for the issue #6

  • Fixed the conditional checks for the uint and int type comparison
  • Fixed the condition where i should be <= _n

Indeed those are typos :)

  • Also saved some gas for the iterative function by using pre-increment operator (++i)

Gas Comparison

Operation Execution Cost i++ 26496 gas ++i 26456 gas

About gas consumption, could you provide an example ? On Remix I tried with severals combinaisons and it results with same gas comsuption.

0xSumitBanik commented 2 years ago

Yes, I mentioned the execution cost which you see on the Remix console. I got different execution cost for using ++i vs i++ in the loop.

i++ image

++i image

Let me know if I'm still confusing you.

wenceslas-sanchez commented 2 years ago

Yes you're right, I was looking at the other method.

mkubdev commented 2 years ago
  • Fixed the conditional checks for the uint and int type comparison
  • Fixed the condition where i should be <= _n

Well spotted!

Also saved some gas for the iterative function by using pre-increment operator (++i)

I can reproduce on Remix! Well spotted too.

@wenceslas-sanchez I think you got same results because you forget to update the <= operators inside the for-loop

wenceslas-sanchez commented 2 years ago

Look good to me. @wenceslas-sanchez ?

Yeah that's perfect !