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

Write methods should check for a missing data argument. #22

Open
acant opened this issue Jun 24, 2019 · 0 comments
Open

Write methods should check for a missing data argument. #22

acant opened this issue Jun 24, 2019 · 0 comments

Comments

@acant
Copy link
Collaborator

acant commented Jun 24, 2019

While using SugarUtils:File.append I accidentally left out the data argument, but I did supply some options:

SugarUtils::FIle.append('dir/filename', owner: 'owner', group: 'group')

Since .append will write out any object which responds to #to_s, it wrote the options hash into the file, did not set owner/group on the file, and did this all silently.

What I had intended to write was:

SugarUtils::FIle.append('dir/filename', 'data_to_append', owner: 'owner', group: 'group')

I think that this parameter pattern will very rarely be valid, so all of the write related methods should check it and raise an exception if:

  • there are only 2 parameters pass to the method
  • the data parameter is a Hash which contains only keys which are optional parameters

This change would mean that:

SugarUtils::FIle.append('dir/filename', owner: 'owner', group: 'group')

but this would not, because it includes an unexpected key:

SugarUtils::FIle.append('dir/filename', owner: 'owner', group: 'group', unexpected_key: :value)

This check would have caught my bug, and I think allow all of the correct calls to the method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant