Conversation
|
Thanks for putting on the pull request, but I've got 2 concerns; the first is the indentation of the function definitions doesn't align with any coding standard I'm familiar to and the second is the change to the split method as this had a pretty hefty gas cost before hand. What's the benefit of the additional while loop? |
|
The indentation is one that comes out of the box using IntelliJ. I suspect you refer in particular to indentation in Strings.sol, right. I'll update this right away. The benefit of the additional while loop is to work around the lack of support for I see that in the alternate PR #6 a different approach has been taken by providing the split array as storage parameter. Not sure that I like this approach, at least not as the only solution. But possibly these two split implementation could complement one another. |
500cb00 to
a82e770
Compare
|
Do you know the gas cost difference with your change e.g. before/after by percentage? |
|
I did some benchmarking with a few different configurations. I used function The first, "new" configuration is identical to the one in In the second, "old" configuration I downgraded Truffle and hence also solc: And naturally in this configuration I used your The result to my surprise shows a decrease of gas cost, from 153246 in the "old" configuration to 63371 in the "new" configuration, i.e. down more than 58%. I reckon that calling Final test I just completed was to stick to your return of To me this suggests going with the fixed size array implementation. |
This PR adds dependency on solc@^0.5.0 a.o. by introducing breaking changes such as
address payable(see breaking changes).An additional branch
feature/truffleexists that on top of this transforms the project into one based on Truffle.