mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 38416: Allow HTTP response codes to be checked with code.
Date Thu, 01 Oct 2015 18:34:46 GMT


> On Sept. 25, 2015, 10:52 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 75-160
> > <https://reviews.apache.org/r/38416/diff/3/?file=1084845#file1084845line75>
> >
> >     Whoa, instead of the giant switch can we keep the map, but avoid the duplicate
information?
> >     
> >     E.g.
> >     
> >     ```
> >     statuses[StatusCode::OK] = "200 OK"
> >     ```
> >     
> >     Possibly even avoiding the need to write 200 here, e.g.
> >     
> >     ```
> >     statuses[StatusCode::OK] = stringify(StatusCode::OK) + " OK"
> >     ```
> 
> Timothy Chen wrote:
>     IMO if all accesses to status code check is just accessing this method, not sure
what's the benefit of keeping the map.
>     I can change it back to the map.

Just to share the motivation, a map seems to fit the problem better: we have to look up information
for the status code. We've typically used switches for differing control flow / logic as opposed
to doing lookup, but it depends on the situation. In this case, it just seems nice to split
the data (map) from the control flow (can we find it in the map?), given the size of the data
we're dealing with.

I imagine you're resistant to the map because of the initialization function it currently
requires? How about we eliminate that by keeping a static heap allocated map that is initialized
using an initializer list?

Also, do you need the explicit casting here?


- Ben


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


On Sept. 25, 2015, 10:38 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, and Jiang
Yan Xu.
> 
> 
> Bugs: MESOS-3429
>     https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp fbd6cf7967173495875a8ea90ed28ade88b982a2

>   3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 97c81b8e61a75771e5bf7d46505cec4e0c0f404a

>   3rdparty/libprocess/src/tests/http_tests.cpp d0b9399d38fa284466a012a21080b1d9007af98b

>   3rdparty/libprocess/src/tests/process_tests.cpp 1023500ac2e3c55be955c4686e7f720eba6b4cc9

> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> -------
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message