-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 .new and .empty? to HTTP::Params #6241
Conversation
src/http/params.cr
Outdated
@@ -291,6 +296,11 @@ module HTTP | |||
raw_params.delete(name) | |||
end | |||
|
|||
# Whether the collection contains any params. | |||
def empty? | |||
raw_params.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use delegate empty?, to: @raw_params
Sorry for edits, I confuse myself :|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact #delegate
is the right thing, and doesn't change the value.
In the API example, the delegate empty?, capitalize, to: @string
changes the string to a capitalized one because a method was specified.
So here, delegate emtpy?, to: @raw_params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I knew that and i use it much in my project. use this because other methods not use delegate. so...i made a mistake 🤣
|
5352c28
to
6a3597c
Compare
I rebase the commit, i think it is better use delegate with |
CI says
|
@bcardiff i found format check error in comments, is that must be formatted? |
Yes, the formatter also formats code in comments. |
src/http/params.cr
Outdated
@@ -76,6 +76,11 @@ module HTTP | |||
end | |||
end | |||
|
|||
# Returns the empty `HTTP::Params`. | |||
def self.new | |||
Params.new({} of String => Array(String)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply add a default to def initialize
, then document that?
Doc should be
Returns an empty
HTTP::Params
.
6e22cd7
to
c104840
Compare
c104840
to
47b0986
Compare
@RX14 Code update, please review again, thanks. |
Add two new methods to
HTTP::Params
:self.new
empty?
Here is no simple way to check if not empty.