search

BlackToppStudios / Mezz_Foundation

Foundational Data types that enforce no opinions on how to build games and provide many facilities that complex games need. This largely avoids graphics and physics, but provides tools like the SortedVector, CountedPtr and HashedString classes.
GNU General Public License v3.0
3 stars 0 forks source link

Proposed SortedManagedArray Port #30

Closed feartheducky closed 6 years ago

feartheducky commented 6 years ago

Ported SortedManagedArray and SortedManagedArrayTests from the monolithic repo.

codecov[bot] commented 6 years ago

Codecov Report

Merging #30 into master will increase coverage by 0.53%. The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   97.56%   98.09%   +0.53%     
==========================================
  Files           4        6       +2     
  Lines         164      210      +46     
==========================================
+ Hits          160      206      +46     
  Misses          4        4
MakoEnergy commented 6 years ago

1) The include guards on binaryfind.h follow the old convention and not the new Jagati convention. Related, binaryfind.h should be BinaryFind.h. 2) The method "add(ElementType value)" around line 160 of SortedManagedArray.h should probably take the element by const reference instead of value. 3) The "Sorter" template parameter on SortedManagedArray is undocumented. It should have a third "@tparam Sorter ..." 4) SortedManagedArray is missing a set of "cbegin", "cend", "crbegin", "crend" iterator methods. (Once implemented they'll need tests written for them) 5) The method "add_emplace(Compare PosFinder, Args&&... Params)" at around line 171 of SortedManagedArray is missing an "@tparam" documentation for Sorter. 6) In the SortedManagedArrayTests file at the end you have an unlabeled code block of additional tests. Their purpose isn't entirely clear. Either the block should be labeled, or a commented added, or both to make the purpose of the additional tests clear. 7) The "}//sortedmanagedarraytests" at the end of SortedManagedArrayTests.h file should be camel cased like the file name, include guard, etc.

Sqeaky commented 6 years ago

I feel bad asking you to wait on this. This is clearly great. This should get merged.

I am curious are all lines shorter than 120 chars?

Even if not this is worth merging.