Code review comment for lp:~gary/juju-gui/bug1090716

The root of the issues here come out to the choice and the reason for it made at the preimplementation given two paths forward. The analogy to me is filtering database content at the app level in each view rendering the given data types instead of abstracting it via db query at either the orm or db level. The principle i'm trying to express is if there is a clear place to implement something once it should be used in preference to implementing it multiple times. The consequence of not doing it is increased costs and fragility because of multiple implementations that need to be kept in sync and duplicated for further display of this data. As an example of that fragility, this implementation leaves out an additional place the filtering logic needs to be cargo copied for correctness in the route for the service. The reason for the pre-implementation choice is specious imo as well given the logic for filtering is behind the utility filtering function, even if it where at the db level, the functionality for the additonal use case presented would be implementable simply via additional parameterization/logic in that utility function.

regarding rework. code is not set in stone, its living and breathing, and rework and refactoring are critical to achieving high quality code. Without it, code decays as the code and feature set evolve without wholistic evaluation.

its unreasonable to expect that reviews only focus on micro concerns, macro concerns are have to be considered fair game for review to achieve the goal of high quality software. Its cheaper in the software development lifecycle to catch issues before they land.

its also unreasonable to expect that a bug/feature request can be so fully specified that any implementation path that leads to working code and tests is considered good. The principle of implementing something once vs multiple times is generally considered obvious to a software engineer, people who implemented this branch have remarked on the same principle in their reviews of other people's work. iotw. its not something that i think is valuable noted in a separate principles page.

Robert's perspective and principles where guided by the situtation and problem he was asked to solve. Namely approaching a multi-million line code base with hundreds of man years spread across dozens of machine and tasked to make it cheaper, faster, and more agile. It also provided a great basis to analyze, measure and derive such principles. Those principles where high-level archiecture oriented though not implementation focused.

The problem domain we're dealing with is much different. Its a small code base and small team and we're evolving practices and knowledge as our feature set and targets grow. The topology rework is being done is a good example of us attempting to incorporate best practices after the fact. Its healthy but time-consuming compared to being able to implement it at the get go.

As a set of principles beyond KISS, i've highlighted this before.
http://gorban.org/post/32873465932/software-architecture-cheat-sheet

its the orthogonal and dry principles that this branch has issues w/ currently.

I'm happy to look over previous rework requests to see if other principles can be derived. but at least in the login one referenced, a pre-implementation call and spec took place and in retrospect may have limited the rework if if had been more detailed.

but we can't expect rework to disappear, and frankly among the current team, it seems we're averaging two a month which seems quite reasonable imo.

all that said, i'm okay with this proceeding without rework inspite of my concerns, given the service route is also updated (ie 3 conditionals), in the hope that's its easy to refactor/remove if its a problem in the future.