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

Add clamp function #545

Merged
merged 1 commit into from
Jan 4, 2016
Merged

Add clamp function #545

merged 1 commit into from
Jan 4, 2016

Conversation

mpolenchuk
Copy link
Contributor

Clamp keeps value within the range.
Employ of soft() makes the whole thing is independant of order.

@tphoney
Copy link
Contributor

tphoney commented Nov 20, 2015

This would also need a readme update. Thanks for the work.

@mpolenchuk
Copy link
Contributor Author

Done

@tphoney
Copy link
Contributor

tphoney commented Nov 20, 2015

Thanks for the quick response 👍

So reading your implementations of clamp i would prefer the parameter ordering of: value, min, max
rather than min, value, max.

I think the function should be limited to 3 values, and not allowing arrays.

@mpolenchuk
Copy link
Contributor Author

Actually the order of params doesn't matter, it could be [max, min, value] or [max, value, min] or ... whatever.
Function technically limited to 3, but it provides flexible input: get arrays, strings as numerics and numerics itself. This was done intentionally.

@mpolenchuk
Copy link
Contributor Author

In other words you might have $range = [minLimit, maxLimit] and valueX.
Just call clamp($range, valueX) and get value within the range.

@mpolenchuk
Copy link
Contributor Author

Bump

else
result << x
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

couldn't this just be args = args.flatten ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, thanx

@DavidS
Copy link
Contributor

DavidS commented Dec 9, 2015

Hi Michael, I added a few comments on issues with the code. I think the overall idea of the clamp function is great, but the code does need a bit work. Especially with the move of puppet 4 to more conscious and strict typing, please reconsider your plan to allow arbitrary arrays and strings as arguments. I would much prefer a more strict signature like tp suggested, that does just take three ints.

Clamp keeps value within the range.
Employ of soft() makes the whole thing is independant of order.
@mpolenchuk
Copy link
Contributor Author

Hi David, thank you for the feedback. I've fixed issues w/ the code.

DavidS added a commit that referenced this pull request Jan 4, 2016
@DavidS DavidS merged commit 9cce930 into puppetlabs:master Jan 4, 2016
@DavidS
Copy link
Contributor

DavidS commented Jan 4, 2016

Thanks for your work, @mpolenchuk !

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

Successfully merging this pull request may close these issues.

3 participants