Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12866 )
Change subject: util: helper class for working with bitsets ...................................................................... Patch Set 4: (8 comments) > Patch Set 3: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h File src/kudu/util/bitset.h: http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@33 PS4, Line 33: compiler 'compile' or 'compilation' http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@40 PS4, Line 40: size_t MaxVals Could MaxVals be deduced based on IntType? IntType stands for the enum's type right? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@46 PS4, Line 46: typedef IntType key_type; Is it necessary to have it in the public section? Maybe, leave it private? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@68 PS4, Line 68: const_iterator I'm curious when it's necessary to have a reference to the 'inserted' element? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@105 PS4, Line 105: size_t size() { const? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@113 PS4, Line 113: IntType& I guess in case of small enums using value instead of reference might be better. http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@131 PS4, Line 131: 'i' What's 'i'? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@184 PS4, Line 184: while (++idx_ < MaxVals) { I'm not sure this is safe, even if this method is private. What happens if accidentally calling this method twice and then trying to use the iterator? -- To view, visit http://gerrit.cloudera.org:8080/12866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0344fe94e9f9da9651164cb1b456c92d99dbdf4 Gerrit-Change-Number: 12866 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 27 Mar 2019 03:51:07 +0000 Gerrit-HasComments: Yes
