Detect body parameter to implement #89 #102
Conversation
| var ret = bodyParams[0]; | ||
| return Tuple.Create(ret.BodyAttribute.SerializationMethod, parameterList.IndexOf(ret.Parameter)); | ||
| } | ||
|
|
||
| if (bodyParams.Count == 0) { |
There was a problem hiding this comment.
We don't need this if anymore -- we've already checked > 1 and == 1.
There was a problem hiding this comment.
We've already got a check for bodyParams.Count > 1 (line 429), then one for bodyParams.Count == 1 (line 434), and both of these throw or return if they are matched.
We don't need to check explicitly for bodyParams.Count == 0 on line 440, since it can't be anything else. (Unless I'm missing something - I am a little slow today 🍻)
There was a problem hiding this comment.
Nope, I was the slow one and already fixed it; the PR is updated ☺
From: Bennor McCarthy [mailto:notifications@github.com]
Sent: Friday, December 19, 2014 6:08 PM
To: paulcbetts/refit
Cc: Oren Novotny
Subject: Re: [refit] Detect body parameter to implement #89 (#102)
In Refit/RequestBuilderImplementation.cshttps://github.com/paulcbetts/refit/pull/102#discussion-diff-22134905:
@@ -423,12 +430,32 @@ string getUrlNameForParameter(ParameterInfo paramInfo)
throw new ArgumentException("Only one parameter can be a Body parameter"); }
// #1, body attribute winsif (bodyParams.Count == 1){var ret = bodyParams[0];return Tuple.Create(ret.BodyAttribute.SerializationMethod, parameterList.IndexOf(ret.Parameter));}
if (bodyParams.Count == 0) {
We've already got a check for bodyParams.Count > 1 (line 429), then one for bodyParams.Count == 1 (line 434), and both of these throw or return if they are matched.
We don't need to check explicitly for bodyParams.Count == 0 on line 440, since it can't be anything else. (Unless I'm missing something - I am a little slow today [:beers:] )
—
Reply to this email directly or view it on GitHubhttps://github.com/paulcbetts/refit/pull/102/files#r22134905.
|
Other than formatting and the redundant Other than that, it looks good to me. |
|
Cool. Now just formatting |
|
Once that ReSharper file is merged in, I can do a reformat with it... |
|
Cool. Maybe we should give @paulcbetts a nudge on #105 - I'm not sure how cool he'd be with me merging my own PR without him at least looking at it. |
No description provided.