It's turtles all the way down.

Or abstractions in this case.

Good practice coding is something that can be infinitely applied, and at some point you're abstracting for the sake of abstracting, which means you've taken it too far. Finding that line is not something that's easy to put into a rule of thumb, as it very much depends on your environment.

For example, we've had customers who were known to ask for simple applications first but then ask for expansions. We've also had customers that ask what they want and generally never come back to us for an expansion.

Your approach will vary per customer. For the first customer, it will pay to pre-emptively abstract the code because you're reasonably certain that you'll need to revisit this code in the future. For the second customer, you may not want to invest that extra effort if you're expecting them to not want to expand the application at any point (note: this doesn't mean that you don't follow any good practice, but simply that you avoiding doing any more than is currently necessary.

How do I know which features to implement?

The reason I mention the above is because you've already fallen in this trap:

But how do I know how much (and what) to parameterize? After all, she might say.

"She might say" is not a current business requirement. It's a guess at a future business requirement. As a general rule, do not base yourself on guesses, only develop what's currently required.

However, context applies here. I don't know your wife. Maybe you accurately gauged that she will in fact want this. But you should still confirm with the customer that this is indeed what they want, because otherwise you're going to spend time developing a feature that you're never going to end up using.

How do I know which architecture to implement?

This is trickier. The customer doesn't care about the internal code, so you can't ask them if they need it. Their opinion on the matter is mostly irrelevant.

However, you can still confirm the necessity of doing so by asking the right questions to the customer. Instead of asking about the architecture, ask them about their expectations of future development or expansions to the codebase. You can also ask if the current goal has a deadline, because you may not be able to implement your fancy architecture in the timeframe necessary.

How do I know when to abstract my code further?

I don't know where I read it (if anyone knows, let me know and I'll give credit), but a good rule of thumb is that developers should count like a caveman: one, two many.

XKCD #764

In other words, when a certain algorithm/pattern is being used for a third time, it should be abstracted so that it is reusable (= usable many times).

Just to be clear, I'm not implying that you shouldn't write reusable code when there's only two instances of the algorithm being used. Of course you can abstract that as well, but the rule should be that for three instances you must abstract.

Again, this factors in your expectations. If you already know that you need three or more instances, of course you can immediately abstract. But if you only guess that you might want to implement it more times, the correctness of implementing the abstraction fully relies on the correctness of your guess.

If you guessed correctly, you saved yourself some time. If you guessed wrongly, you wasted some of your time and effort and possibly compromised your architecture to implement something you end up not needing.

If destroyCity(City city) is better than destroyBaghdad(), is takeActionOnCity(Action action, City city) even better? Why / why not?

That very much depends on multiple things:

• Are there multiple actions that can be taken on any city?


• Can these actions be used interchangeably? Because if the "destroy" and "rebuild" actions have completely different executions, then there's no point in merging the two in a single takeActionOnCity method.

Also be aware that if you recursively abstract this, you're going to end up with a method that's so abstract that it's nothing more than a container to run another method in, which means you've made your method irrelevant and meaningless.

If your entire takeActionOnCity(Action action, City city) method body ends up being nothing more than action.TakeOn(city);, you should wonder if the takeActionOnCity method truly has a purpose or isn't just an extra layer that adds nothing of value.

But taking actions on cities is not enough for me, how about takeActionOnGeographicArea(Action action, GeographicalArea GeographicalArea)?

The same question pops up here:

• Do you have a use case for geographical regions?


• Is the execution of an action on a city and a region the same?


• Can any action be taken on any region/city?

If you can definitively answer "yes" to all three, then an abstraction is warranted.

Practice

This is Software Engineering SE, but crafting software is a lot more art than engineering. There's no universal algorithm to follow or measurement to take to figure out how much reusability is enough. Like with anything, the more practice you get designing programs the better you will get at it. You'll get a better feel for what is "enough" because you'll see what goes wrong and how it goes wrong when you parameterize too much or too little.

That's not very helpful now though, so how about some guidelines?

Look back at your question. There's a lot of "she might say" and "I could". A lot of statements theorizing about some future need. Humans are shite at predicting the future. And you (most likely) are a human. The overwhelming problem of software design is trying to account for a future you don't know.

Guideline 1: You Ain't Gonna Need It

Seriously. Just stop. More often than not, that imagined future problem doesn't show up - and it certainly won't show up just like you imagined it.

Guideline 2: Cost/Benefit

Cool, that little program took you a few hours to write maybe? So what if your wife does come back and ask for those things? Worst case, you spend a few more hours tossing together another program to do it. For this case, it's not too much time to make this program more flexible. And it's not going to add much to the runtime speed or memory usage. But non-trivial programs have different answers. Different scenarios have different answers. At some point, the costs are clearly not worth the benefit even with imperfect future telling skills.

Guideline 3: Focus on constants

Look back at the question. In your original code, there's a lot of constant ints. 2018, 1. Constant ints, constant strings... They're the most likely things to need to be not-constant. Better yet, they take only a little time to parameterize (or at least define as actual constants). But another thing to be wary of is constant behavior. The System.out.println for example. That sort of assumption about use tends to be something that changes in the future and tends to be very costly to fix. Not only that, but IO like this makes the function impure (along with the timezone fetching somewhat). Parameterizing that behavior can make the function more pure leading to increased flexibility and testability. Big benefits with minimal cost (especially if you make an overload that uses System.out by default).

Firstly: No security minded software developer would write a DestroyCity method without passing an Authorisation Token for any reason.

I too can write anything as an imperative which has evident wisdom without it being applicable in another context. Why is it necessary to authorise a string concatenation?

Secondly: All code when executed must be fully specified.

It does not matter whether the decision was hard coded in place, or deferred to another layer. At some point there is a piece of code in some language that knows both what is to be destroyed and how to instruct it.

That could be in the same object file destroyCity(xyz), and it could be in a configuration file: destroy {"city": "XYZ"}", or it might be a series of clicks and keypresses in a UI.

Thirdly:

Honey.. Can you print all the Day Light Savings around the world for 2018 in the console? I need to check something.

is a very different set of requirements to:

she wants to pass a custom string formatter, ... interested in only certain month periods, ... [and] interested in certain hour periods...

Now the second set of requirements obviously makes for a more flexible tool. It has a broader target audience, and a broader realm of application. The danger here is that the most flexible application in the world is in fact a compiler for machine code. It is literally a program so generic it can build anything to make the computer whatever you need it to be (within the constraints of its hardware).

Generally speaking people who need software do not want something generic; they want something specific. By giving more options you are in fact making their lives more complicated. If they wanted that complexity, they would instead be using a compiler, not asking you.

Your wife was asking for functionality, and under-specified her requirements to you. In this case it was seemingly on purpose, and in general it's because they don't know any better. Otherwise they would have just used the compiler themselves. So the first problem is you didn't ask for more details about exactly what she wanted to do. Did she want to run this for several different years? Did she want it in a CSV file? You didn't find out what decisions she wanted to make herself, and what she was asking you to figure out and decide for her. Once you've figured out what decisions need to be deferred you can figure out how to communicate those decisions through parameters (and other configurable means).

That being said, most clients miss-communicate, presume, or are ignorant of certain details (aka. decisions) that they really would like to make themselves, or that they really didn't want to make (but it sounds awesome). This is why work methods like PDSA (plan-develop-study-act) are important. You've planned the work in line with the requirements, and then you developed a set of decisions (code). Now it's time to study it, either by yourself or with your client and learn new things, and these inform your thinking going forward. Finally act on your new insights - update the requirements, refine the process, get new tools, etc... Then start planning again. This would have revealed any hidden requirements over time, and proves progress to many clients.

Finally. Your time is important; it is very real and very finite. Every decision you make entails many other hidden decisions, and this is what developing software is about. Delaying a decision as an argument may make the current function simpler, but it does make somewhere else more complex. Is that decision relevant in that other location? Is it more relevant here? Whose decision is it really to make? You are deciding this; this is coding. If you repeat sets of decision frequently, there is a very real benefit in codifying them inside some abstraction. XKCD has a useful perspective here. And this is relevant at the level of a system be it a function, module, program, etc.

The advice at the start implies that decisions your function has no right to make should be passed in as an argument. The problem is that a DestroyBaghdad function might actually be the function that has that right.

Experience, Domain Knowledge, and Code Reviews.

And, regardless of how much or how little experience, domain knowledge, or team you have, you cannot avoid the need to refactor as-needed.

With Experience, you'll start to recognize patterns in the domain-nonspecific methods (and classes) you write. And, if you're at all interested in DRY code, you'll feel bad feelings when you're about a write a method that you instinctively know you'll write variations of in the future. So, you'll intuitively write a parametrized least common denominator instead.

_{(This experience may transfer over instinctively into some your domain objects and methods too.)}

With Domain Knowledge, you'll have a sense for which business concepts are closely related, which concepts have variables, which are fairly static, etc..

With Code Reviews, under- and over-parametrization will more likely be caught before it becomes production code, because your peers will (hopefully) have unique experiences and perspectives, both on the domain and coding in general.

That said, new developers won't generally have these Spidey Senses or an experienced group of peers to lean on right away. And, even experienced developers benefit from a basic discipline to guide them through new requirements — or through brain-foggy days. So, here's what I'd suggest as a start:

• Start with the naive implementation, with minimal parametrization.
  _{(Include any parameters you already know you'll need, obviously ...)}
  


• Remove magic numbers and strings, moving them to configs and/or parameters


• Factor "large" methods down into smaller, well-named methods


• Refactor highly redundant methods (if convenient) into a common denominator, parametrizing the differences.

These steps don't necessarily occur in the stated order. If you sit down to write a method you already know to be highly redundant with an existing method, jump straight into refactoring if it's convenient. (If it's not going to take significantly more time to refactor than it would be to just write, test, and maintain two methods.)

But, apart from just having lots of experience and so forth, I advise pretty minimalistic code DRY-ing. It's not hard to refactor obvious violations. And, if you're too zealous, you can end up with "over-DRY" code that's even harder to read, understand, and maintain than the "WET" equivalent.

There's a lot of long winded answers here, but honestly I think it's super simple

Any hard coded information you have in your function that isn't part
of the function name should be a parameter.

so in your function

class App {

    void dayLightSavings() {

        final Set<String> availableZoneIds = ZoneId.getAvailableZoneIds();

        availableZoneIds.forEach(zoneId -> {

            LocalDateTime dateTime = LocalDateTime.of(LocalDate.of(2018, 1, 1), LocalTime.of(0, 0, 0));

            ZonedDateTime now = ZonedDateTime.of(dateTime, ZoneId.of(zoneId));

            while (2018 == now.getYear()) {

                int hour = now.getHour();

                now = now.plusHours(1);

                if (now.getHour() == hour) {

                    System.out.println(now);

                }

            }

        });

    }

}

You have:

The zoneIds

2018, 1, 1

System.out

So I would move all these to parameters in one form or another. You could argue that the zoneIds are implicit in the function name, maybe you would want to make that even more so by changing it to "DaylightSavingsAroundTheWorld" or something

You don't have a format string, so adding one is a feature request and you should refer your wife to your family Jira instance. It can be put on the backlog and prioritised at the appropriate project management committee meeting.

In short, don't engineer your software for reusability because no end user cares if your functions can be reused. Instead, engineer for design comprehensibility -- is my code easy for someone else or my future forgetful self to understand? -- and design flexibility -- when I inevitably have to fix bugs, add features, or otherwise modify functionality, how much will my code resist the changes? The only thing your customer cares about is how quickly you can respond when she reports a bug or asks for a change. Asking these questions about your design incidentally tends to result in code that is reusable, but this approach keeps you focused on avoiding the real problems you will face over the life of that code so you can better serve the end user rather than pursuing lofty, impractical "engineering" ideals to please the neck-beards.

For something as simple as the example you provided, your initial implementation is fine because of how small it is, but this straightforward design will become hard to understand and brittle if you try to jam too much functional flexibility (as opposed to design flexibility) into one procedure. Below is my explanation of my preferred approach to designing complex systems for comprehensibility and flexibility which I hope will demonstrate what I mean by them. I would not employ this strategy for something that could be written in fewer than 20 lines in a single procedure because something so small already meets my criteria for comprehensibility and flexibility as it is.

Objects, not Procedures

Rather than using classes like old-school modules with a bunch of routines you call to execute the things your software should do, consider modeling the domain as objects which interact and cooperate to accomplish the task at hand. Methods in an Object-Oriented paradigm were originally created to be signals between objects so that Object1 could tell Object2 to do its thing, whatever that is, and possibly receive a return signal. This is because the Object-Oriented paradigm is inherently about modeling your domain objects and their interactions rather than a fancy way to organize the same old functions and procedures of the Imperative paradigm. In the case of the void destroyBaghdad example, instead of trying to write a context-less generic method to handle the destruction of Baghdad or any other thing (which could quickly grow complex, hard to understand, and brittle), every thing that can be destroyed should be responsible for understanding how to destroy itself. For example, you have an interface that describes the behavior of things that can be destroyed:

interface Destroyable {

    void destroy();

}

Then you have a city which implements this interface:

class City implements Destroyable {

    @Override

    public void destroy() {

        ...code that destroys the city

    }

}

Nothing that calls for the destruction of an instance of City will ever care how that happens, so there is no reason for that code to exist anywhere outside of City::destroy, and indeed, intimate knowledge of the inner workings of City outside of itself would be tight coupling which reduces felxibility since you have to consider those outside elements should you ever need to modify the behavior of City. This is the true purpose behind encapsulation. Think of it like every object has its own API which should enable you to do anything you need to with it so you can let it worry about fulfilling your requests.

Delegation, not "Control"

Now, whether your implementing class is City or Baghdad depends on how generic the process of destroying the city turns out to be. In all probability, a City will be composed of smaller pieces that will need to be destroyed individually to accomplish the total destruction of the city, so in that case, each of those pieces would also implement Destroyable, and they would each be instructed by the City to destroy themselves in the same way someone from outside requested the City to destroy itself.

interface Part extends Destroyable {

    ...part-specific methods

}



class Building implements Part {

    ...part-specific methods

    @Override

    public void destroy() {

       ...code to destroy a building

    }

}



class Street implements Part {

    ...part-specific methods

    @Override

    public void destroy() {

        ...code to destroy a building

    }

}



class City implements Destroyable {

    public List<Part> parts() {...}



    @Override

    public void destroy() {

        parts().forEach(Destroyable::destroy);            

    }

}

If you want to get really crazy and implement the idea of a Bomb that is dropped on a location and destroys everything within a certain radius, it might look something like this:

class Bomb {

    private final Integer radius;



    public Bomb(final Integer radius) {

        this.radius = radius;

    }



    public void drop(final Grid grid, final Coordinate target) {

        new ObjectsByRadius(

            grid,

            target,

            this.radius

        ).forEach(Destroyable::destroy);

    }

}

ObjectsByRadius represents a set of objects that is calculated for the Bomb from the inputs because the Bomb does not care how that calculation is made so long as it can work with the objects. This is reusable incidentally, but the main goal is to isolate the calculation from the processes of dropping the Bomb and destroying the objects so you can comprehend each piece and how they fit together and change the behavior of an individual piece without having to reshape the entire algorithm.

Interactions, not Algorithms

Instead of trying to guess at the right number of parameters for a complex algorithm, it makes more sense to model the process as a set of interacting objects, each with extremely narrow roles, since it will give you the ability to model the complexity of your process through the interactions between these well-defined, easy to comprehend, and nearly unchanging objects. When done correctly, this makes even some of the most complex modifications as trivial as implementing an interface or two and reworking which objects are instantiated in your main() method.

I'd give you something to your original example, but I honestly can't figure out what it means to "print... Day Light Savings." What I can say about that category of problem is that any time you are performing a calculation, the result of which could be formatted a number of ways, my preferred way to break that down is like this:

interface Result {

    String print();

}



class Caclulation {

    private final Parameter paramater1;



    private final Parameter parameter2;



    public Calculation(final Parameter parameter1, final Parameter parameter2) {

        this.parameter1 = parameter1;

        this.parameter2 = parameter2;

    }



    public Result calculate() {

        ...calculate the result

    }

}



class FormattedResult {

    private final Result result;



    public FormattedResult(final Result result) {

        this.result = result;

    }



    @Override

    public String print() {

        ...interact with this.result to format it and return the formatted String

    }

}

Since your example uses classes from the Java library which don't support this design, you could just use the API of ZonedDateTime directly. The idea here is that each calculation is encapsulated within its own object. It makes no assumptions about how many times it should run or how it should format the result. It is exclusively concerned with performing the simplest form of the calculation. This makes it both easy to understand and flexible to change. Likewise, the Result is exclusively concerned with encapsulating the result of the calculation, and the FormattedResult is exclusively concerned with interacting with the Result to format it according to the rules we define. In this way, we can find the perfect number of arguments for each of our methods since they each have a well-defined task. It's also much simpler to modify moving forward so long as the interfaces don't change (which they aren't as likely to do if you've properly minimized the responsibilities of your objects). Our main() method might look like this:

class App {

    public static void main(String args) {

        final List<Set<Paramater>> parameters = ...instantiated from args

        parameters.forEach(set -> {

            System.out.println(

                new FormattedResult(

                    new Calculation(

                        set.get(0),

                        set.get(1)

                    ).calculate()

                )

            );

        });

    }

}

As a matter of fact, Object-Oriented Programming was invented specifically as a solution to the complexity/flexibility problem of the Imperative paradigm because there is just no good answer (that everyone can agree on or arrive at independently, anyhow) to how to optimally specify Imperative functions and procedures within the idiom.

There's a clear process you can follow:

• Write a failing test for a single feature which is in itself a "thing" (i.e., not some arbitrary split of a feature where neither half really makes sense).


• Write the absolute minimum code to make it pass green, not a line more.


• Rinse and repeat.


• (Refactor relentlessly if necessary, which should be easy due to the great test coverage.)

This turns up with - at least in the opinion of some people - pretty much optimal code, since it is as small as possible, each finished feature takes as little time as possible (which might or might not be true if you look at the finished product after refactoring), and it has very good test coverage. It also noticeably avoids over-engineered too-generic methods or classes.

This also gives you very clear instructions when to make things generic and when to specialize.

I find your city example weird; I would very likely never ever hardcode a city name. It is so obvious that additional cities will be included later, whatever it is you're doing. But another example would be colors. In some circumstances, hardcoding "red" or "green" would be a possibility. For example, traffic lights are such an ubiquitous color that you can just get away with it (and you can always refactor). The difference is that "red" and "green" have universal, "hardcoded" meaning in our world, it is incredibly unlikely that it will ever change, and there is not really an alternative either.

Your first daylight savings method is simply broken. While it conforms to the specifications, the hardcoded 2018 is particularly bad because a) it is not mentioned in the technical "contract" (in the method name, in this case), and b) it will be out of date soon, so breakage is included from the get-go. For things that are time/date related, it would very seldomly make sense to hardcode a specific value since, well, time moves on. But apart from that, everything else is up for discussion. If you give it a simple year and then always calculate the complete year, go ahead. Most of the things you listed (formatting, choice of a smaller range, etc.) screams that your method is doing too much, and it should instead probably return a list/array of values so the caller can do the formatting/filtering themselves.

But at the end of the day, most of this is opinion, taste, experience and personal bias, so don't fret too much about it.

I've come to the opinion that there are two sorts of reusable code:

• Code which is reusable because it's such a fundamental, basic thing.


• Code which is reusable because it has parameters, overrides and hooks for everywhere.

The first sort of reusability is often a good idea. It applies to things like lists, hashmaps, key/value stores, string matchers (e.g. regex, glob, ...), tuples, unification, search trees (depth-first, breadth-first, iterative-deepening, ...), parser combinators, caches/memoisers, data format readers/writers (s-expressions, XML, JSON, protobuf, ...), task queues, etc.

These things are so general, in a very abstract way, that they're re-used all over the place in day to day programming. If you find yourself writing special-purpose code that would be simpler if it were made more abstract/general (e.g. if we have "a list of customer orders", we could throw away the "customer order" stuff to get "a list") then it might be a good idea to pull that out. Even if it doesn't get re-used, it lets us decouple unrelated functionality.

The second sort is where we have some concrete code, which solves a real issue, but does so by making a whole bunch of decisions. We can make it more general/reusable by "soft-coding" those decisions, e.g. turning them into parameters, complicating the implementation and baking in even more concrete details (i.e. knowledge of which hooks we might want for overrides). Your example seems to be of this sort. The problem with this sort of reusability is that we may end up trying to guess at the use-cases of other people, or our future selves. Eventually we might end up having so many parameters that our code isn't usable, let alone reusable! In other words, when calling it takes more effort than just writing our own version. This is where YAGNI (You Ain't Gonna Need It) is important. Many times, such attempts at "reusable" code end up not being reused, since it may be incompatable with those use-cases more fundamentally than parameters can account for, or those potential users would rather roll their own (heck, look at all the standards and libraries out there whose authors prefixed with the word "Simple", to distinguish them from the predecessors!).

This second form of "reusability" should basically be done on an as-needed basis. Sure, you can stick some "obvious" parameters in there, but don't start trying to predict the future. YAGNI.

A good rule of thumb is: your method should be as reusable as… reusable.

If you expect that you will call your method only in one place, it should have only parameters that are known to the call site and that are not available to this method.

If you have more callers, you can introduce new parameters as long as other callers may pass those parameters; otherwise you need new method.

As the number of callers may grow in time, you need to be prepared for refactoring or overloading. In many cases it means that you should feel safe to select expression and run “extract parameter” action of your IDE.

Ultra short answer: The less coupling or dependency to other code your generic module has the more reusable it can be.

Your example only depends on

import java.time.*;

import java.util.Set;

so in theory it can be highly reusable.

In practise i donot think that you will ever have a second usecase that needs this code so following yagni principle i would not make it reusable if there are not more than 3 different projets that need this code.

Other aspects of reusability are ease of use and doocumentation which corelate with Test Driven Development: It is helpfull if you have a simple unit-test that demonstrates/documents an easy use of your generic module as a coding example for users of your lib.