Programming Practice - using requires with return types

Question

Can I please get some help with the following? Is it acceptable practice to allow my functions to rely on require statements "passing" to return the correct value? In the example below if the function does not revert due to the require statements triggering, return true.

I understand that time calculation is not precise in Solidity and I'm using SafeMath. start and end are contract variables.

Example Function:

// ensure the current duration is valid.
function isValidDuration()
    public
    view
    returns (bool)
{
    uint32 calculatedDuration = uint32(end.sub(start));

     require(
        calculatedDuration >= 15 minutes,
        "The minimum session duration is 15 minutes."
    );

    require(
        calculatedDuration <= 1 weeks,
        "The maximum session duration is 1 week."
    );

    return true;
}

Answer 1

Your code doesn't make much sense.

The function either returns true or throws an exception, so what exactly is the point in returning a single possible value? One could simply conclude that if the function hasn't thrown an exception then the answer is true.

---

With regards to the actual question about using require:

Revert (embedded in require) is useful only when the function changes the state of the contract, because it allows you to revert those changes if anything "goes wrong".

But your function doesn't change the state of the contract, so there is nothing to revert here.

Other than that, the function seems to be a kind of a "boolean getter", so why not simply return true when 15 minutes <= calculatedDuration <= 1 week and false otherwise?

You can reduce it to:

uint256 calculatedDuration = end.sub(start);
return 15 minutes <= calculatedDuration && calculatedDuration <= 1 week;

---

UPDATE:

The answer above was made under the assumption that this function was designated to be used off-chain (i.e., from a web3 script).

If you actually intended for it to be used on-chain (i.e., from this contract or from another contract), then the common practice is to implement it as a modifier (which doesn't return anything of course):

modifier isValidDuration() {
    uint256 calculatedDuration = end.sub(start);
    require(calculatedDuration >= 15 minutes, "The minimum duration is 15 minutes");
    require(calculatedDuration <= 1 weeks, "The maximum duration is 1 week");
    _;
}

Answer 2

In most cases, I would avoid require in a check like that to make it safe to check.

if(duration < min) return false;
if(duration > max) return false;
return true;

A caller that is considering a state change can

require(contract.durationIsValid());

That is easily incorporated into a modifier if that is wanted, and the intricacies of the logic would be codified in one place only.

Hope it helps.