> On Aug. 13, 2013, 2:15 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/linkedhashmap.hpp, lines > > 15-16 > > <https://reviews.apache.org/r/13464/diff/4/?file=340073#file340073line15> > > > > Can you move these typedefs down into the private block where they are > > used? > > Vinod Kone wrote: > They are used in the public methods. FWIW, this is the sytle followed by > stout::cache and boost.
Ah so they have to be here? > On Aug. 13, 2013, 2:15 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/linkedhashmap.hpp, lines > > 59-69 > > <https://reviews.apache.org/r/13464/diff/4/?file=340073#file340073line59> > > > > Not yours but hashmap returns hashsets for keys and values, can you add > > a TODO or update hashmap to return lists? values() should definitely not be > > returning a hashset there (that's totally broken since hashmaps can contain > > duplicate values). Not sure how much you want to fix so TODO(vinod) or > > TODO(bmahler) is fine if you want to delegate. :) > > > > I like what you have here, we could optionally add the following to > > both LinkedHashMap and hashmap, akin to python: > > > > hashset<Key> keyset() const; > > > > This would also unbreak code that relies on hashmap.keys() returning a > > hashset. > > Vinod Kone wrote: > Added a TODO in hashmap. I don't see a reason for returning hashsets in > linkedhashmap because the whole point of it is get keys and values in the > insertion order. If we need it later for some reason, I prefer to add it then. Sounds good, if/when we add keyset() to hashmap, we should add it here as well. hashmap and LinkedHashMap should be compatible, with the only difference being that the iteration order / key order is different. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13464/#review25049 ----------------------------------------------------------- On Aug. 13, 2013, 11:34 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13464/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2013, 11:34 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am > 0cd407cfbd3226ce610ef77cee3a16c0e198e144 > 3rdparty/libprocess/3rdparty/stout/Makefile.am > e465fd1b7c22a649a59e69a29c0c7f3e9198188b > 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp > 796cb505fe538d5f65c0f8ea1fa827155f2bb2c1 > 3rdparty/libprocess/3rdparty/stout/include/stout/linkedhashmap.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp > PRE-CREATION > > Diff: https://reviews.apache.org/r/13464/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
