Clojure

Add error checking for defmulti options.

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

It would be nice to have some error checking for option map in macros like defmulti. I've written a check-options function that can be used for that purpose and used it in defmulti definition in the attached patch.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Assembla Importer added a comment -

Converted from http://www.assembla.com/spaces/clojure/tickets/236
Attachments:
0001-Added-the-check-options-function-to-warn-about-wrong.patch - https://www.assembla.com/spaces/clojure/documents/cD6KBS90Kr3RGweJe5afGb/download/cD6KBS90Kr3RGweJe5afGb

Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/236 Attachments: 0001-Added-the-check-options-function-to-warn-about-wrong.patch - https://www.assembla.com/spaces/clojure/documents/cD6KBS90Kr3RGweJe5afGb/download/cD6KBS90Kr3RGweJe5afGb
Hide
Permalink
Assembla Importer added a comment -

richhickey said: Thanks for this. Should throw IllegalArgumentException I think.

Show
Assembla Importer added a comment - richhickey said: Thanks for this. Should throw IllegalArgumentException I think.
Hide
Permalink
Stuart Halloway added a comment -

Rich, I have a few design questions before I edit this patch:

  • For macros throwing IllegalArgumentException looks right, but I would want basically the same fn as a precondition assertion in normal code.
  • To the extent that the previous argument (or any other kind of option checking) is persuasive, I am reluctant to give out the generic name "check-options" in core without more thought. Maybe make this private for now? Or call it check-macro-options? Put in a different namespace?
Show
Stuart Halloway added a comment - Rich, I have a few design questions before I edit this patch:
  • For macros throwing IllegalArgumentException looks right, but I would want basically the same fn as a precondition assertion in normal code.
  • To the extent that the previous argument (or any other kind of option checking) is persuasive, I am reluctant to give out the generic name "check-options" in core without more thought. Maybe make this private for now? Or call it check-macro-options? Put in a different namespace?
Hide
Permalink
Rich Hickey added a comment -

I don't understand

Show
Rich Hickey added a comment - I don't understand
Hide
Permalink
Stuart Halloway added a comment -

I have two ideas for this patch:

  1. the thrown error tells you what you should have done (the valid keys) but not what you did (the keys you passed). Should do both.
  2. the name check-options. There might be an opposite fn (make sure you did provide keys) that could claim the same name. Maybe constrain-options? Or, to the extend that it works with any map, could be constrain-keys. ... Which suggests the partner fn require-keys.

Thoughts?

Show
Stuart Halloway added a comment - I have two ideas for this patch:
  1. the thrown error tells you what you should have done (the valid keys) but not what you did (the keys you passed). Should do both.
  2. the name check-options. There might be an opposite fn (make sure you did provide keys) that could claim the same name. Maybe constrain-options? Or, to the extend that it works with any map, could be constrain-keys. ... Which suggests the partner fn require-keys.
Thoughts?
Hide
Permalink
Stuart Halloway added a comment -

Rich: we had discussed making the validation helper a local fn, but I like the idea of making it private instead. I expect there will be other uses in the future. Hope that is ok. If not, let me know and I will make the patch we discussed.

Show
Stuart Halloway added a comment - Rich: we had discussed making the validation helper a local fn, but I like the idea of making it private instead. I expect there will be other uses in the future. Hope that is ok. If not, let me know and I will make the patch we discussed.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.