https://codereview.appspot.com/547980044/diff/572060043/lily/include/skyline.hh File lily/include/skyline.hh (right):
https://codereview.appspot.com/547980044/diff/572060043/lily/include/skyline.hh#newcode34 lily/include/skyline.hh:34: #include "offset.hh" Please don't separate the lily includes - actually interval.hh is already included above https://codereview.appspot.com/547980044/diff/572060043/lily/skyline.cc File lily/skyline.cc (right): https://codereview.appspot.com/547980044/diff/572060043/lily/skyline.cc#newcode28 lily/skyline.cc:28: #include "skyline.hh" As discussed in other reviews, skyline.hh should be the very first include and system headers should be last https://codereview.appspot.com/547980044/diff/572060043/lily/skyline.cc#newcode171 lily/skyline.cc:171: class BuildingQueue Do you really need a custom type for this? It really much looks like an iterator (and you can call methods on the current item by using the -> operator) https://codereview.appspot.com/547980044/diff/572060043/lily/skyline.cc#newcode175 lily/skyline.cc:175: Building front_; In any case, this causes copies on every call to next(). If you need to retain this data structure, is there a reason not to use a pointer to the current element? AFAICS you don't modify the underlying vector when using a BuildingQueue https://codereview.appspot.com/547980044/diff/572060043/lily/skyline.cc#newcode365 lily/skyline.cc:365: class LessThanBuilding Can we just replace this functor by passing Building::less as a function pointer? https://codereview.appspot.com/547980044/
