Skip to content

Conversation

@waterlink
Copy link
Collaborator

fixes #167

Implemented first option:

Contract (1..10) => String
def do_something(range)

@waterlink
Copy link
Collaborator Author

/cc @egonSchiele @gsinclair

@gsinclair
Copy link
Contributor

Nice. What happens if you pass a string, say, to a 1..10 argument? And what should happen?

@alex-fedorov
Copy link
Collaborator

Good question. Will add spec for that ;)

@alex-fedorov
Copy link
Collaborator

Well, as I think about it further, what should happen? One of:

  • ContractError: Expected: 1..10; Actual: "hello world" - this is how it works now;
  • ContractError: Expected: Range(Numeric); Actual: String - this is nice, but what to do if you have a range of some weird type that has inheritance and stuff like this, how to determine base type?

WDYT?

@gsinclair
Copy link
Contributor

I think how it works now is fine. Worth making a spec though.

On Wed, Jul 22, 2015 at 10:21 PM, Alexey Fedorov [email protected]
wrote:

Well, as I think about it further, what should happen? One of:

  • ContractError: Expected: 1..10; Actual: "hello world" - this is how it works now;
  • ContractError: Expected: Range(Numeric); Actual: String - this is nice, but what to do if you have a range of some weird type that has inheritance and stuff like this, how to determine base type?
    WDYT?

    Reply to this email directly or view it on GitHub:
    Add Range validator #184 (comment)

@waterlink
Copy link
Collaborator Author

@gsinclair I actually have added spec for this before adding my last comment ;)

egonSchiele added a commit that referenced this pull request Jul 22, 2015
@egonSchiele egonSchiele merged commit fc98f41 into egonSchiele:master Jul 22, 2015
@egonSchiele
Copy link
Owner

thanks!

@waterlink waterlink deleted the feature/add-range-as-a-contract branch July 22, 2015 16:43
@abevoelker
Copy link
Contributor

Ooh I was just looking for a Range contract last night - nice timing and thank you!

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

Successfully merging this pull request may close these issues.

Add range as a contract

5 participants