Peter & David
What we did
In the course of the session we looked at
- The cost of Debt - while all pairs in the session had the same feature to add to a conceptually "simple" piece of JavaScript, sneakily half the pairs were given a superlative piece of easy-to-understand code, and the other half a much more realistic real-world example of code that had evolved under changing requirements
- Recognising Debt - looking at some Java code to identify different kinds of code liability (see outputs below)
- Forensic accounting - we looked at the history of the 'bad' code from the first exercise to try to understand some of the assumptions and decisions that had led to it's sorry state
- Balancing the books - going back to the categories of debt identified in "Recognising Debt", groups came up with proposals for avoiding these particular kinds of debt
- Auditing - we talked as a group about tools and practices which you can introduce to your teams to get them thinking about issues of code debt and how to fix them - notably recrimination-free refactoring (where the team works on a simple piece of code they're not responsible for, to foster a sense of pleasure in fixing crummy code) and a build and reflect exercise, where pairs from your team tackle a simple development task over a more extended period, and then come together to compare design decisions and reflect on why some are better - in avoiding debt - than others.
Outputs
Problems in systems that aren't simply errors can be put broadly into one of three categories: Correctness (fidelity to requirements, robustness, stability), Efficiency (with respect to time, space, scalability) and Clarity. Although all can carry liabilities, the Code Debt which we'#re particularly concerned with is in this last area - to do with the code communicating to its human readers, subcategorised further
- Comprehensibility
- Simplicity (minimal, no baggage)
- Conceptual integrity (correspondence with domain, effective use of language and design idioms)
- Behavioural integrity (no hidden order dependencies on method calls, for example - often to do with state)
Comprehensibility
Liabilities identified
- Interface documentation missing
- Interface name too abstract
- No description of how or why an abstract class should be subclassed
- Magic numbers in code
- Magic numbers expected as arguments
- Uncommunicative class name
- Misleading method name
- In particular, a method named for what it does to the internals of a class, rather than from the client's point of view
- Meaningless parameter names to method
- Misleading comment
- Out-of-date comment
- No class comment
- Methods with similar names doing very different things
- Use accurate names
- Give everything a name (i.e. avoid 'i', 'j')
- Follow the domain in naming and behaviour
- Use language consistently
- Declare and use domain-specific types and aliases (was "minimize use of built-in types")
- Understand, share and follow common coding idioms
- Refactoring (e.g. if a name isn't clear, don't ask permission, change it!)
- Avoid comments [i.e. avoid the need for comments. DH]
- Explain exceptions [presumably in the sense of 'special cases', not programming language exceptions. DH]
- Collective code review
Simplicity
Liabilities identified
- Interface with way too many methods
- Interface/abstract class combining behaviours that should be separated
- Class with lots of code - source file too long
- Complex constructor with lots of flags
- Refactor!
- Use tools that support refactoring
- Make time to refactor
- Factor out common code, give it a meaningful name
- Generate and act on code metrics
- Test Driven Development
- Write down debt as it is incurred or found
- Peer code review - share good practice
- Share and stick to good coding/design conventions
- Use appropriate design idioms/patterns
- Keep a class's responsibilities simple and coherent
Conceptual integrity
Liabilities identified
- Interface/abstract class combining behaviours that should be separated
- State (e.g. Boolean flag) used to distinguish behaviours that should be separated out into distinct classes
- Confusing inheritance
- Inappropriate (or indeed any) use of instanceOf
- Knowledge of subtypes in supertypes
- Take naming seriously
- Name and structure code to reflect the problem domain
- Talk about names
- Describe the code intention in prose [spoken, written? DH] before implementing
- Make your tests tell a story about what the code is supposed to do
- Refactor (again!) - have courage, touch everything that needs it
- Be an abstractionist
- Establish and use a metaphor for common understanding
- Identify and separate concerns in the code
- If you hear someone say "I'm going to add to ... " - beware! [i.e. there are often many better ways to enhance a design than adding to the responsibilities or implementation of a single class or component/ DH]
Behavioural integrity
Liabilities identified
- Subclasses that behave differently when passed the same invalid arguments
- Method that does more than it's named to do
- Methods that depend on order of call. May even be documented, but not enforced
- Classes that don't fully or correctly implement their interfaces
- Bertrand Mayo [sic! I'll have one with Tuna, to go... DH] Shorthand for understanding and documenting pre- and post-conditions and invariants
- Use intention-revealing names to minimise inappropriate use of these methods
- When implementing an interface, implement all of it.
- Don't allow clients to rely on more than the specification [though the spec had better be good! DH]
- Don't try to solve a behavioural problem without understanding it
David's reflections
We all seem to understand comprehensibility and to an extent simplicity, but searching for the core of conceptual and behavioural integrity seems harder. Conceptual is about understanding the syntax and semantics of both the domain and the implementation technology, and marrying these understandings in a way that doesn't distort or obfuscate either. Behavioural integrity seems to be the hardest to grok (though in fairness perhaps there weren't that many good examples of this in the code we reviewed) - but things like forced initialisation protocols, race conditions, and ability to set state (and break encapsulation and invariants) all play a part.
