Starting with:
Not know what I am going to break with my changes.
Ideally you'd want a very high level of confidence about what you're about to break; to favor "quick and dirty", a lower, but still reasonable, level of confidence may be acceptable. Instead of reading every line of code until you fully understand all of it, you could try:
Trust your toolchain : IDE / editor.
Just about any modern text editor is going to have "find all" instead of just "find", and many of the ones meant for code will have something like "find all within project", or "find all within workspace". Worst case scenario, you fall back on something like grep in the source directory. If the list of occurrences of the name is small, you can start by focusing on just the code around those uses of the name, making notes as you go regarding program flow around it.
Some IDEs can do additional validation. They might automatically find the bits they think you need to refactor, and let you approve/reject each one. If you're targeting an embedded chip, for example, they might include code generation tools for peripheral configuration, and validate the configuration for you, telling you if you tried to use an impossible configuration.
Trust your toolchain : build after each minor change to check for errors
The language may have constructs which can help. Depending on your code base, an incremental build with a minor change may be (much) faster than trying to read over the code yourself. In c++, for example, you can start to find out what will break by changing, for the part of the code you need to modify, any of the following (one at a time):
- make one previously non-const method or reference/pointer parameter const, so you can see which other calls expected a non-const reference/pointer, and examine which ones actually modify the object (vs nobody considered const correctness), and which ones don't need to impact your reasoning about its state, to limit how much you need to know about what you're changing
- make a previously public (or protected) data member private (tells you something about what might break if you change the data type, or want to enforce preconditions or postconditions?)
- change a pointer parameter to a reference parameter (tells you something about what might break if nullptr isn't allowed)
- make an integral type with macro defined values into an enum class (tells you : who might break if I add or remove possible values; most compilers will also give you better warnings about unhandled cases in switch statements)
- where other constraints are meant to apply to a type (for example as indicated via comments on function parameters or return values), make a new type that enforces those constraints. For example, if a floating point input or output is not allowed to be NaN or infinity, you could make a thin wrapper class that throws an exception (or uses an alternate default value) if constructed with one of those values. Initially the converting constructor and conversion operator can be explicit so you can see where in the code those assumptions were being made, but if you're satisfied that they're ok, or just want to try it and see if it breaks, you can make them implicit instead.
- enforce your assumptions via compile-time assertions when possible. Recall that even in Test-Driven Development, "failing to compile" counts as a failing unit test, and static assertions can be the simplest and fastest tests to add. For example,
//TODO: fix this for platforms where float is too large be copied into 32 bits becomes
static_assert(sizeof(float)*CHAR_BIT <= 32u, "ASSUMPTION VIOLATION: float cannot be stored in 32 bits"); If placed near the relevant code, it becomes easier to reason about that code, too ("what happens if float is too large? becomes "we know the float is small enough if this compiles").
Trust your toolchain : tactically enable/disable warnings.
Many compilers have different warning levels that can hint at code that is likely either already broken, or likely to break in the face of any changes; in new code, false positives are less likely, so all warnings can be enabled all the time, but in legacy code, this can bury useful warnings about new code in a mountain of warnings on old code that aren't likely to be a problem (or may not be fixable).
The last time I built anything for Windows CE, for example, the number of warnings Microsoft's compiler issued for their own system headers dwarfed the warnings in even the legacy code, let alone my changes. Fortunately, compilers also tend to have some (typically non-portable) means to conditionally disable warnings, maybe one individual type of warning, or all warnings temporarily around included headers (like system headers), or even completely disabled except fully enabled around your new code.
Do be aware that warnings are only emitted for the files being compiled, so you may need a clean build to see relevant warnings.
Trust your toolchain : sanitizers / static analysis.
Some of these can pretty dramatically increase build times (I've seen this for example with gcc and libasan), so you might consider setting up both a "sanitized debug" build target (uses these tools) and a "rapid debug" build target (does not). This lets you switch between a slower build that checks more possible sources of bugs for you, and one that won't make you wait so long when your changes are minimal.
Trust history: Review change logs for mentions of the thing you are trying to modify
If you're lucky, the legacy code will already be under version control. If you're luckier, the commit messages there will provide useful information about what had to change and why--similar to engineering change control--rather than just being snapshots at a particular time.
My personal experience has been that even legacy code outside of version control usually at least has some sort of change log; a text file in the project, comments at the top of the main file, etc. If you can find a previous change that involved the same part of the program, you've got a head start on where to look. If the change log is only a summary and doesn't describe which parts had to change, you may be able to find an older version (developers of programs not kept in version control sometimes retain copies of older source versions) and use a diff tool to find the changes.
Next, onto:
Not know how code is structured and have to read the whole thing prior to writing
Once again, instead of waiting for full understanding of everything, for "quick and dirty" we're going to try to reach a lower, but acceptable, level of confidence. Specifically, instead of reading the entire code base, we're going to target what to read first by making educated guesses.
Educated guesses: Familiarize yourself with domain jargon and abbreviations
The names that are unclear to you may become obvious if you know the appropriate jargon.
"FFT" for example is just three letters, but in signal processing code, it most likely means "Fast Fourier Transform", and a function called FFT(...) is therefore probably what you might have named "calculate_fast_fourier_transform(...)". It is even more likely to be an FFT if it is being used near names like LPF (low-pass filter), FIR (finite impulse response), etc. You might have already known those ones, but the less familiar you are with a problem domain, the less likely you are to be familiar with the relevant jargon.
In my experience, employers will be happy that you want to learn more about their industry and may at your request provide some resources for you to do so; perhaps trade magazines, site visits, textbooks, introductory classes, etc.
Educated guesses: Familiarize yourself with common idioms in your language
Languages tend to have idioms besides the usual OOP runtime polymorphism and design patterns.
In c++, for example, compile-time polymorphism might look weird if you aren't used to templates. Functional languages will look differently, too.
This extends to short names, too. In c++, for example, if you see "itr", that's most likely an object that satisfies one of the iterator categories or concepts, whereas in other languages it might be an object that implements an "iEnumerator" interface, for example.
Educated guesses: Identify (possibly undocumented) patterns in the names
Most programmers develop habits in naming, even if it is not found in a style guide and/or is not always followed. Do all caps mean something different than all lower case? Does camel case mean something different than lower case with underscores? Is there a common prefix or postfix for globals, members, type names? Do some names resemble names from something else (another programming language's standard functions, mathematical set theory, etc)?
Even names that aren't immediately obviously may have patterns that let you make educated guesses.
Leverage your toolchain: navigation and readability
This one isn't about guessing, and you might already know these, but make sure you are making use of features your editor provides. Modern editors usually have things to make reading easier, via context menus or hotkeys, like:
- context-sensitive "go to implementation", "go to declaration", etc.
- backward and forward navigation to/from prior cursor positions or the context-sensitive jumps
- splitting large files into two or more views with independent seek positions
- iterative macro expansion
Then, we come to:
Start to build encapsulations myself
Others have already suggested various forms of "write less". So, how and why can we reach an acceptable, but less than "clean", level of encapsulation?
Familiarize yourself with the reasoning behind the rules
I don't mean a generic "it will save time in the long run". Actual, specific kinds of changes it makes faster to implement. Actual, specific types of defects that are prevented.
For example, consider the answers about how to iterate an enum in c++:
Some of the answers suggest putting the enum values into a container (vector, initializer_list, set) and iterating the container. Some suggest making a custom iterator type. Some suggest, for enums with continuous values, using sentinel values in the enum itself and incrementing the underlying value.
The "sentinel" version is simplest and fastest to write, but less robust against change in some specific ways. Specifically: it will be easier to miss updating a loop somewhere when adding a new value, making the iteration miss a valid value. Specifically: if you remove a value, leaving a "hole" in the interval, the iteration will attempt to use an invalid value. Specifically: if you write the underlying value to a file somewhere for serialization, an added value could replace a sentinel value, meaning the new code may interpret old files incorrectly.
The other methods reduce the potential future maintenance burden for some or all of those issues, but take more time today to implement and/or are more complicated and/or have performance impacts and/or make any code that wants to iterate dependent on the container type (suppose you started with std::set before realizing boost::flat_set would have been better).
Generally the clean approach is more robust against unexpected change, and thus makes a reasonable default choice, but how likely is it that the values will change for the specific enum, in the specific use case, of a specific program?
Understanding the specific problems the rules are meant to prevent is crucial to deciding when you can break them with little risk of suffering significant consequences.
Make use of business logic for simplifying assumptions
If you are picking unit systems to display road vehicle speed, for example, you can limit yourself to km/hr and mi/hr, using a simple flag to track which one should be displayed (you could use something like boost::units to do the actual calculations).
A more robust design could allow for multiple unit systems, perhaps letting the user make up any unit system they wanted, having an adapter pattern for changing them between internal representations and display, some way to uniquely identify each unit system that is encapsulated to avoid leaking its internal representation, etc.
But, if the business case is to display road vehicle travel speed, that additional abstraction and complexity doesn't provide any additional value; no application other than silly jokes would want a road vehicle speed readout in decameters per day or millimeters per microsecond. So, we make the simplifying assumption: exactly two unit systems, which will not change. A boolean flag would be adequate, though if you really wanted, a two-value enumeration with more descriptive names (but no container or iterator, please) would be appropriate.
That case is familiar to those of us who drive, but there are other opportunities in other business cases that may not be immediately obvious to the casual observer. So, get to know the business use of your software.
Beyond asking your employer generally for resources to gain industry knowledge, technical sales people -- specifically, sales people who have spent time doing the actual work in the industry ("gotten their hands dirty"), not just a salesperson with "technical" in their title -- can be a good option. In addition to familiarity with customer needs, they (usually) have a different perspective from your developer colleagues, and are less likely to have any stake in the politics of code development.
Leverage cost to identify what can (currently) be left out
Keeping with the speed unit example, if you'd been building some industrial process control device instead of an automobile, you might start with in/min (USA) and cm/min (others) initially instead of mi/hr and km/hr. In the case of industrial process controls, it is more likely that you'd encounter a customer who wants another option, say cm/s, or m/min, but do they want it badly enough to wait extra time if you already had the other two working?
In my experience, people--including customers, clients, managers, salespeople, and even other software engineers--will ask for anything and everything they think is free (to them). So, make sure they know it isn't free for them. On teams with formalized schedules--true for both waterfall and Agile--the time cost comes out as a result of the schedule, but in less formal circumstances you might have to just point out the time (and any other) costs yourself. If the perceived business value doesn't exceed that cost, the customer / manager / salesperson may cut the unnecessary work for you.
Cut time on application code before you cut time on libraries
Since we're trying to choose specific tradeoffs that provide more benefits than costs, it is good to remember that the benefits and costs for code in a shared library are not necessarily the same as the benefits and costs for code in an application.
Firstly, the application itself is a kind of "encapsulation", in that changes to objects and interfaces wholly contained within just that application (as opposed to output data, shared static libraries, or library APIs) shouldn't break other applications, limiting the costs of breaking things.
Secondly, time spent on code wholly contained in just an application can only benefit that application, limiting the benefit of not breaking things, whereas code in a library has the chance to benefit multiple applications.
As an example, the non-virtual interface (NVI) pattern has some nice benefits, and for a reusable library, I'd be inclined to use it myself. But for application code, changing the interface functions later from virtual to non-virtual ones that call virtual implementations may not be as painful a change as it would be in a reusable library, because it probably doesn't break as many things, so one of the benefits of NVI is reduced. For an unambiguous interface that has no "steps" to perform, and for which you don't even expect to need debug / logging hooks, another expected benefit of NVI is less. So, the cost/benefit comparison of NVI vs a simpler pure abstract implementation may come out differently for a library than for application code, depending on the specific class involved.
In some cases, composition may be preferable to inheritance, even in a language like c++ that allows multiple, virtual, public vs private inheritance. And, again, if you're writing re-usable library code, it may well make sense; maybe even template your class on the composed type so you can substitute in different but similar types (I appreciate, for example, that boost::flat_map is designed that way). It may also make sense to stick with composition if the existing code is already written following data-oriented design. But if you're solving one specific problem in one specific part of one specific application by wrapping an existing class in a new type that enforces additional constraints, private inheritance may get it done faster than composition. "using private_base_class::function_name;" on the member functions that are not concerned with the new constraints is less code to write than writing boilerplate wrappers for a dozen functions and their overloads; less boilerplate to scan during code reviews; and, fewer lines of insignificant code to distract new developers looking for the bits that are interesting. Maybe you still pick composition, but do so having considered the tradeoffs in that specific situation.
Consider compile-time abstractions instead of run-time abstractions
Particularly for application code, maybe also for static library code, but maybe not for shared library code, unless it is wholly contained within the library.
In c++ for example, instead of writing your own fancy gang-of-four iterator to abstract a container implementation, consider whether you can't just use a type alias to refer to the iterator of the container implementation you're using today. If the container does change in the future, the alias definition can change to match, letting the compiler do the work of changing it for you when you rebuild. You'll have more to recompile, and other objects will still know about that implementation detail, but you won't have to edit much source code yourself, and the existing iterators are already written, tested, and compatible with the already written and tested standard library algorithms; that's a lot of code you don't have to write.
Not everything needs an interface
Not everything needs to be abstracted behind a reference or pointer to an interface.
For example, if some function in your code takes screen pixel coordinates, it's ok for it to be public knowledge that those are integers, and it is ok to use them as a value type. If you work with screen pixel positions / counts, having two integers is a natural part of that interface. You can put them in a std::pair<std::uint_least16_t, std::uint_least16_t>, or your own simple structure, but you don't need to hide the fact that there are integers in there, nor which kind of integer they are. You don't necessarily need to take 32 bits of numeric data and hide it behind a potentially 64-bit pointer that needs a vtable lookup to get the actual data you already know you need.
Can I promise that there will never be a screen more than 2^16-1 pixels wide? Of course not; we've seen how that kind of hubris played out with IPv4, 32-bit timestamps, etc. But we already scale pixel buffers that are different sizes than the display resolution, so the specific case of screen sizes increasing is not likely to be as significant a problem as those examples.
This can also be true for objects more complicated than "plain data". Writing, testing, and maintaining interfaces for every class, whether or not it has multiple implementations, is overkill for application code.
Consider simplicity over abstraction if they would be at odds
I've seen non-robotic (that is, non-automomous), industrial machinery, that runs on what was, printed out, less than half a page of BASIC (including comments); basically a simple state machine built on gotos. It was not robust. Initially, for example, the software de-bouncing was very limited because it relied on filtering in the sensor hardware; after the obsolete original sensor was replaced with one having a noisier signal, the state machine had to be changed to add more robust bounce transitions.
But with just half a page needed to understand the entire program, that program was easier to maintain than some more robust programs that were larger and more complicated. I wasn't there when that program was first written, but at half a page I'd bet it was pretty darn quick to write, too.
Some larger, complicated programs aren't ever going to fit half a page, and benefit from encapsulation and abstraction. But, it would have been a mistake to add a bunch of abstraction to that half page program to make it more robust, because it was already so simple that it would have been a net drain to do so.
Don't fall victim to "Not Invented Here" syndrome
Is there a tested, reputable, appropriately-licensed library that implements most of the functionality you want, only not quite the way you wanted it?
Maybe they use a data structure, algorithm, or API that isn't what you prefer, but unless it is unacceptable for some reason, it may be faster to use it than to write, test, and maintain your own.
You don't have to fix everything right now
Others have already noted that there's room to improve encapsulation in existing code without cleaning up everything, and a small improvement may be enough for the change you are trying to make for the next release. You can fix more on the release after that, if it ever even happens; as the Agile advocates have noted, priorities often change, and by the time a full fix would be prioritized, you might be working somewhere else, or the application may be rewritten from the ground up, or the company may no longer be in business. Until then, consider small improvements:
- Maybe add const qualifiers around just the thing you're changing if it didn't have them, instead of adding const-correctness to the entire interface.
- Maybe make the thing you're changing private if it used to be public, instead of making all data members private by default in all objects.
- Maybe add an RAII wrapper around one resource whose lifetime is changing, if it used to be manually managed, instead of adding RAII to every resource right off the bat.
- Maybe inject differing concrete behavior with a functor instead of making a whole new abstract interface class with different implementations; existing code can keep taking references to the existing concrete class without caring which functor it was given on construction.
- Instead of changing lots of tightly-coupled objects, maybe write one adapter/facade/decorator thin shim layer to decouple them from new/changed code, so the existing code can be left as it is (for now).
Finally, we come to:
Writing ... tested ... code
Others have already suggested various forms of "test less"; after all, the quickest way to test is not to. How, and why, can we find an amount of testing that is acceptable, but less than that of "clean" code?
Test based on risk analysis and cost/benefit tradeoff
Tests have a cost--time to write, time to validate, time to perform, time to maintain--and hopefully a benefit in the increased confidence that the code is correct (even in the face of potential changes).
Consider, for example, the following c++ function:
constexpr std::uint64_t wide_sum(std::uint32_t lhs, std::uint32_t rhs) noexcept
{
return static_cast<std::uint64_t>(lhs) + static_cast<std::uint64_t>(rhs);
}
Now, in c++, if types uint64_t and uint32_t exist (via #include ), they are exactly 64 and exactly 32 bits, respectively. The mathematical interval of [0,2^64-1] can always contain the sum of any two values both in the interval [0, 2^32-1]. Both 32 bit values are widened into 64 bits before the summation via static_cast, so the summation is performed in 64 bits and returned as 64 bits, so there is no chance for it to overflow. We trust the built-in operator+ as much as we trust the rest of the compiler. We already have about as high a level of confidence as we can get; if this function compiles at all (maybe there's a typo, maybe the target platform doesn't support those fixed-width integers, maybe we forgot to include cstdint, etc), it's going to be right (barring compiler bugs). Spending any additional time to write a test for it (as opposed to just using "compilation success" as the test) is not going to provide any additional value.
More generally, what and how thoroughly to test comes down to risk assessment. Except in applications with significant hazards or regulatory requirements, you probably don't need to spend time on a risk matrix or exhaustively following a standard for risk assessment, but it's usually not bad to think about, before deciding how thoroughly to test:
- How bad is it if this code is wrong? (is it likely to cause physical harm? will it cause service infrastructure to fail? is it likely to result in a lawsuit? is it likely to lose sales? is it likely to only be somewhat embarrassing?)
- How likely is it that the code is wrong (is this a trivial function I can think up an informal proof about, or, is it using an algorithm that is not as well understood, does it have high cyclomatic complexity, does it involve implementation-defined behavior, etc?)
- What else is done to mitigate possible errors (is there a redundant calculation performed another way, with the results serving as a run-time check on each other? Is the user allowed to override the result themselves if they need? Does the system as a whole put itself into some "known, acceptably safe" state when it detects errors like memory corruption, access violations, etc?)
- How likely is it that this code will change (did we incur some technical debt with an incomplete implementation we know we'll need to rewrite later? did we make assumptions in which we aren't very confident? is our business case well-understood and our industry stable, or is this a rapidly changing industry, or one still figuring out best practices, etc?)
Some situations may justify having significantly more test code than product code, but don't start out assuming that every situation does; think through whether less would be appropriate.
Find obvious problems quickly : don't be afraid to just try it (safely, in a mock environment)
Don't push questionable code to production, but that doesn't mean you can't safely test questionable code in something closely resembling its environment. Maybe you've got a simulator or an emulator, or can mock up your own test hardware. For example, a program that controls the motion of a plasma cutting torch might be tested without any torch or plasma by putting a marker or pen where the plasma torch would normally be clamped, with paper taped to the work surface so you can see the path it has drawn.
Sure, one quick "does it even look close to right" test doesn't give you much confidence that the code isn't wrong, but if it makes an obvious error quickly, you might have spent five minutes finding that you have a problem in the position feedback loop instead of spending hours or days trying to exhaustively figure out what to test and how. In particular this can be useful if you're making one change to otherwise working software and can compare the output of both versions.
Depending on your risk assessment, you may find a way to adequately test your changes on emulators / mock hardware instead of implementing a whole testing suite in a codebase that didn't previously have one.
So which one is it? Maybe the issue of your sleep needs to be addressed before everything else – Helena Apr 10 '22 at 09:19