Is going the void route gonna be easier with c++11? If yes, I would prefer void over Nothing.
@vinodkone Sent from my mobile On Sep 11, 2012, at 7:15 PM, "Benjamin Hindman" <[email protected]> wrote: > > >> On Sept. 11, 2012, 9:32 p.m., Benjamin Hindman wrote: >>> This definitely needs to get done, so I'm stoked you took it on! However, >>> this is the kind of thing I think merits some discussion (before >>> unnecessary work is done ... sorry). In particular, I had created >>> 'Try<void>' some time ago for this exact reason, but didn't use it after >>> thinking we might want to use 'Try<Nothing>' instead. Here were the pros I >>> saw to using 'Try<void>': >>> >>> + It captures the "void" return type well. ;) >>> + We can eliminate 'Try<void>::get' so that people can't even attempt to >>> get something that doesn't exist (although, a 'get' on a Try<Nothing> >>> returns an object that you can't really do anything with, so it's very >>> harmless). >>> >>> However, there were also cons: >>> >>> - The 'Try<void>' implementation is mostly duplicated code. >>> - You have to do 'return Try<void>::some();' which doesn't read as nice as >>> it could (at least, not as nice as 'return Nothing();'). >>> - To do the same thing for Result and Future will require lots of >>> duplicated code, which is at least a non-starter for Future and thus we'll >>> probably always be using Future<Nothing> for asynchronous cases (and it >>> seems much cleaner to be consistent). >>> >>> For these reasons, I was slightly more inclined towards 'Try<Nothing>'. >>> Naturally, I'd love to hear others thoughts! >> >> Ben Mahler wrote: >> You've brought up some good points, I would agree with killing Try<void> >> entirely and switching this to do Try<Nothing> instead, does that sound good? >> >> What I originally wanted was just non-templatized Try instead of >> Try<void>, but again that requires the duplicated code and likely returning >> a messy Try::some() rather than Nothing(). > > Well, Try<Nothing> is my vote, so if nobody else has any input, I say go for > it. > > > - Benjamin > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7001/#review11357 > ----------------------------------------------------------- > > > On Sept. 11, 2012, 5:05 p.m., Ben Mahler wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/7001/ >> ----------------------------------------------------------- >> >> (Updated Sept. 11, 2012, 5:05 p.m.) >> >> >> Review request for mesos, Benjamin Hindman, Vinod Kone, and Jie Yu. >> >> >> Description >> ------- >> >> We unnecessarily have Try<bool>s all over the place, these are tri-state: >> {error, some:true, some:false}. It seems most cases, we never use >> {some:false} in the function or the caller. >> >> So, this restores some sanity to use two-state Try<void>s: {error, some} >> >> >> Diffs >> ----- >> >> src/linux/cgroups.hpp 1a3cdc2 >> src/linux/cgroups.cpp 53d611f >> src/linux/fs.hpp 31a6100 >> src/linux/fs.cpp 744aea6 >> src/logging/logging.cpp d6d31ec >> src/slave/cgroups_isolation_module.hpp 00255b5 >> src/slave/cgroups_isolation_module.cpp 8a121e0 >> src/slave/gc.hpp 3760d09 >> src/slave/gc.cpp 5212a41 >> src/slave/process_based_isolation_module.cpp c0576bd >> src/slave/slave.cpp 4ea1db1 >> src/tests/cgroups_tests.cpp fbaa046 >> src/tests/configurator_tests.cpp 8baed76 >> src/tests/files_tests.cpp 6ef2004 >> src/tests/stout_tests.cpp f690fac >> src/tests/zookeeper_server.hpp 4f34910 >> src/webui/webui.cpp d4f2ab9 >> third_party/libprocess/include/stout/os.hpp 602db1f >> third_party/libprocess/src/process.cpp 2d2b56c >> >> Diff: https://reviews.apache.org/r/7001/diff/ >> >> >> Testing >> ------- >> >> osx 10.7 gcc 4.2.1 >> redhat Red Hat 4.1.2-48 gcc 4.1.2 >> >> make >> make check >> >> note that SampleFrameworks.PythonFramework is consistently failing on red >> hat, unrelated to this change >> >> >> Thanks, >> >> Ben Mahler >> >> >
