Skip to content

Add KeywordArgs and Optional contracts #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 22, 2015

Conversation

waterlink
Copy link
Collaborator

Fixes #131

Allows to do this:

Contract OptHash[nfiles: Opt[Nat], mfiles: Nat] => ArrayOf[String]
def list(nfiles: 10, mfiles:)
  []
end

# both of these work as expected
list(nfiles: 4, mfiles: 77)
list(mfiles: 77)

# and this will fail with contract error as expected as opposed to `Maybe[Nat]` solution
list(mfiles: 77, nfiles: nil)

Contract names are subject to discussion.

@waterlink waterlink force-pushed the add-opt-hash-contract branch from 1807f83 to 7616b40 Compare May 19, 2015 11:24
@nixpulvis
Copy link
Contributor

I'd call the contract Options personally.

@waterlink
Copy link
Collaborator Author

@nixpulvis thought about that too, but the chance that the user will have the Options class in their domain is much higher (just my gut feeling, maybe not)

@nixpulvis
Copy link
Contributor

Yea, I actually hit this with existing contracts pretty often anyway. I think the real solution is namespacing these contracts under something like C::Options.

@waterlink
Copy link
Collaborator Author

So how about KeywordArgs name then? WDYT?
On May 19, 2015 8:38 PM, "Nathan Lilienthal" [email protected]
wrote:

Yea, I actually hit this with existing contracts pretty often anyway. I
think the real solution is namespacing these contracts under something like
C::Options.


Reply to this email directly or view it on GitHub
#151 (comment)
.

@alex-fedorov
Copy link
Collaborator

/cc @egonSchiele @sfcgeorge

```

```
ContractError: Contract violation for argument 2 of 2:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just before this ContractError, can you add a line that calls the connect function that will throw the related ContractError, and make it obvious that we are not passing in a value for port?

@egonSchiele
Copy link
Owner

Left comments, looks good otherwise. 👍 for updating the tutorial. KeywordArgs sounds better to me since that reflects where this contract will be used..

@alex-fedorov
Copy link
Collaborator

@egonSchiele Addressed.

Plus renamed both contracts to KeywordArgs[ ..a-hash.. ] and Optional[ ..a-contract.. ].

@alex-fedorov alex-fedorov changed the title Add OptHash and Opt contracts Add KeywordArgs and Optional contracts May 22, 2015
egonSchiele added a commit that referenced this pull request May 22, 2015
Add KeywordArgs and Optional contracts
@egonSchiele egonSchiele merged commit e227cda into egonSchiele:master May 22, 2015
@egonSchiele
Copy link
Owner

Thanks!

@sfcgeorge
Copy link
Contributor

Nice!

@gsinclair
Copy link
Contributor

Excellent!

If we're discussing names, my choice is NamedArgs and Opt. I think brevity is a good aim: contract specs can get quite long.

@egonSchiele
Copy link
Owner

Hey @gsinclair, we already merged this as KeywordArgs.

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.

Problem with default keyword arguments
6 participants