https://codereview.appspot.com/547980044/diff/548010043/lily/skyline.cc File lily/skyline.cc (right):
https://codereview.appspot.com/547980044/diff/548010043/lily/skyline.cc#newcode348 lily/skyline.cc:348: result.push_back (buildings->at (0)); On 2020/04/21 04:25:05, Dan Eble wrote: > at() involves a boundary check that is not necessary here. front() or [0] would > be better. Done. https://codereview.appspot.com/547980044/diff/548010043/lily/skyline.cc#newcode862 lily/skyline.cc:862: // testing On 2020/04/21 04:25:05, Dan Eble wrote: > I love the idea of testing, though I think we'll want to find a way to avoid > bloating LilyPond with test code. I agree that this approach is unorthodox, but I there is little alternative. The data structure under test is a smob, so testing without setting up the GUILE infrastructure will be hard. For the amount of unittests we currently have (ie. none), it is a lot of overhead to setup an extra compile, integrate the testing into the build, and keep the test framework in sync with the main code's GUILE integration. We could compile enclose the code with #ifndef NDEBUG and use the same mechanism that we use to suppress assert statements (although I think those are shipped in production code too?) If we start having lots of unittests (which would be great!) we can consider splitting of unittests is worth it. https://codereview.appspot.com/547980044/