Skip to content
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

API change suggestion on set(key, value[, clobber=true]) #1

Open
elmasse opened this issue Mar 20, 2014 · 3 comments
Open

API change suggestion on set(key, value[, clobber=true]) #1

elmasse opened this issue Mar 20, 2014 · 3 comments

Comments

@elmasse
Copy link

elmasse commented Mar 20, 2014

Just a small thing, and you don't have to agree with me but, I consider that in these cases it would be more readable an API with a third param as an options object like:

set(key, value[, {clobber: true}]);

since when in code you know what that false/true means:

c.set('a-HEADER', 'more', {clobber: false});

What do you think?

@mikeal
Copy link
Member

mikeal commented Mar 20, 2014

It just feels a little weird to have an options object when there is only one option. I'm trying to think of another case where we might need an option though.

@elmasse
Copy link
Author

elmasse commented Mar 21, 2014

@mikeal while I agree that it feels weird I think it is a positive change in terms of code readability. I, as most developers, read docs when I'm trying to use a certain feature, but when reading someone else's code I usually guess by the API what the method is doing.
So, in this particular case, I would expect a key, value params in a setter method, but a third param as boolean is not intuitive at all.

So, just my two cents, feel free to close this issue if you think it is useless.

Best,
Max

@mikeal
Copy link
Member

mikeal commented Mar 21, 2014

hrm... i think you're right, i just need to think about the best way to do this without breaking API or killing performance.

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

No branches or pull requests

2 participants