> On May 5, 2016, 8:49 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 616-617
> > <https://reviews.apache.org/r/46504/diff/2/?file=1369337#file1369337line616>
> >
> >     Why not just use a lambda?
> >     
> >     ```cpp
> >       MethodNotAllowed(
> >           const std::initializer_list<std::string>& allowMethods,
> >           const std::string& requestMethod)
> >         : Response(
> >               [&allowMethods, &requestMethod]() {
> >                 std::string body = "Expecting ";
> >                 if (allowedMethods.size() == 1) {
> >                   body += "'" + allowedMethods.begin()[0] + "'";
> >                 } else if (allowedMethods.size() == 2) {
> >                   body += "'" + allowedMethods.begin()[0] + " or " + "'" +
> >                           allowedMethods.begin()[1] + "'";
> >                 } else {
> >                   std::initializer_list<std::string>::iterator it;
> >                   std::size_t ind;
> >                   for (it = allowedMethods.begin(); it != 
> > allowedMethods.end();
> >                        ++it) {
> >                     ind = it - allowedMethods.begin();
> >                     if (ind == allowedMethods.size() - 1) {
> >                       body += "or '" + *it + "'";
> >                     } else {
> >                       body += "'" + *it + "', ";
> >                     }
> >                   }
> >                 }
> >                 body += ", received '" + requestMethod + "'";
> >                 return body;
> >               }(),
> >               Status::METHOD_NOT_ALLOWED) {}
> >     ```
> >     
> >     If you want to define it out-of-line, why not just a private static 
> > function?
> >     
> >     ```cpp
> >       MethodNotAllowed(
> >           const std::initializer_list<std::string>& allowMethods,
> >           const std::string& requestMethod)
> >         : Response(
> >             createBody(allowMethods, requestMethod),
> >             Status::METHOD_NOT_ALLOWED) {}
> >             
> >       private:
> >       
> >       static std::string createBody(
> >           const std::initializer_list<std::string>& allowMethods,
> >           const std::string& requestMethod) {
> >         std::string body = "Expecting ";
> >         if (allowedMethods.size() == 1) {
> >           body += "'" + allowedMethods.begin()[0] + "'";
> >         } else if (allowedMethods.size() == 2) {
> >           body += "'" + allowedMethods.begin()[0] + " or " + "'" +
> >           allowedMethods.begin()[1] + "'";
> >         } else {
> >           std::initializer_list<std::string>::iterator it;
> >           std::size_t ind;
> >           for (it = allowedMethods.begin(); it != allowedMethods.end();
> >                ++it) {
> >             ind = it - allowedMethods.begin();
> >             if (ind == allowedMethods.size() - 1) {
> >               body += "or '" + *it + "'";
> >             } else {
> >               body += "'" + *it + "', ";
> >             }
> >           }
> >         }
> >         body += ", received '" + requestMethod + "'";
> >         return body;
> >       }
> >     ```
> >     
> >     In either case, we wouldn't need the `MethodNotAllowedBody` class nor 
> > the `MethodNotAllowed(MethodNotAllowedBody member);` constructor.

Having lambda is too cryptic, but since we got rid of the state in 
`MethodNotAllowed` we can easily replace it with a static function.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46504/#review131919
-----------------------------------------------------------


On May 3, 2016, 12:20 a.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46504/
> -----------------------------------------------------------
> 
> (Updated May 3, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4126
>     https://issues.apache.org/jira/browse/MESOS-4126
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Constructed error string in MethodNotAllowed.
> 
> - Decided to use constructor delegation rather than just constructing in 
> MethodNotAllowed
>   allowing body to be constructed before calling the Response constructor. 
> This in addition
>   to the improved readability and sectioning off the verbose body 
> construction as a middle step
>   in a delegation chain led me to this implementation. Noting here that 
> construction in 
>   MethodNotAllowed would be completely valid, but seeing what others think of 
> this choice.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 8f4eabcbb71ead1f5c28e1d8a2dd40db7af1f297 
> 
> Diff: https://reviews.apache.org/r/46504/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Reply via email to