kleros / openiico-contract

Contract of Interactive Coin Offering
MIT License
11 stars 5 forks source link

function search returns(uint nextInsert) ➡️ but we are returning value in the last line anyway #34

Closed stefek99 closed 6 years ago

stefek99 commented 6 years ago

https://github.com/kleros/openiico-contract/blob/master/contracts/IICO.sol#L277-L295

clesaege commented 6 years ago

Yeah, what is the problem?

stefek99 commented 6 years ago

Isn’t it a duplication?

You ether “return value”

Or have a function signature “returns something”

Similar to #28 when I didn’t know the syntax.

clesaege commented 6 years ago

You can also name the variable you return for better documentation. I don't know if it actually creates it for nothing. But normally those kind of stuff are solved at the compiler level.

stefek99 commented 6 years ago

I still don't get it.

nextInsert does not appear anywhere in the code, other than function signature:

image

We return return next; here https://github.com/kleros/openiico-contract/blob/3c57d7d6ebcd8be9fb7bae9aa4b8d22bec57603c/contracts/IICO.sol#L294

clesaege commented 6 years ago

Yeah, nextInsert is just the function signature, it could have been omitted from the compiler perspective but it was put there in order for readers to have better clarity about what the function returns.

stefek99 commented 6 years ago

Let me be blatantly straight.

Because in Western culture people are so fucking polite:

It's OK to admit a mistake. It's OK to admit copy-paste error. It's OK to change your mind. It's OK to realize the facts came into light etc...

Wish someone told me these in my relationship.

Wish there were more strong leaders like Putin - try saying "no" to Putin LOL 😎

image

better clarity

Let me be blatantly straight.

In #28 I've learned about syntactic sugar of Solidity - nice - you can have returns in the function signature, so no need to return at the end of the function body.

So now I know the syntax, I know what it is doing, I can do the returns and then I don't need to return. This time around it's confusing (as opposed to "better clarity").

Did some reading - https://ethereum.stackexchange.com/questions/34046/what-is-the-purpose-of-giving-name-to-return-variables-in-function-declaration - they mention visual aid...

I'll still lean towards consistency.

(hope you still like me, 6 months ago I coudl barely follow a tutorial how to deploy a ERC20 and ICO, this time around I'm on a mission to make crypto more usable to the people)

(hope the IICO model will be used and reused that's why #32 so people who sent some ether will have aligned incentives to promote the project)

#decentralizeeverything

clesaege commented 6 years ago

Yeah, as you saw on the readings, it's for visual aid to understand better what the function returns.

stefek99 commented 6 years ago

That at least deserves a compiler warning...

pragma solidity ^0.4.23;
contract ThatReturnsSomething {

    function Foo(uint a, uint b) pure public returns (uint sum) {
        sum = a + b;
    }

    // TypeError: Different number of arguments in return statement than in returns declaration.
    // function Bar(uint a, uint b) public {
    //     return a + b;
    // }

    function Baz(uint a, uint b) pure public returns (uint sum) {
        return a * b; 
    }

    function Foo2(uint a, uint b) pure public returns (uint sum) {
        sum = a + b;
        uint somethingElse = 123; // something random
        return somethingElse;
    }
}