Flag arguments
Software development, especially good practices, are always a matter of contextual intent. If you describe the same thing but stress its description differently, one description can "feel" wrong and the other can "feel" right.
The direct answer to your question is yes, having your consumer knowingly branch your method's sublogic is a bad approach. The consumer should not steer the implementation.
This is a flag argument. It often manifests as a boolean, but your enum is doing the same thing, it represents a set of flag arguments.
But, and this is key, having your consumer pass a value, and unbeknownst to them this passed value ends up being using by your method in a way that it happens to call a different (sub)method, is perfectly fine. Here, the consumer only provided some info, and the implementation is the one who steered itself.
The difference between these two is the implication that the consumer knows what is happening behind the scenes based on this value. They shouldn't. That's the point of encapsulation: only the implementation needs to know how the implementation works and what it does.
Maybe an example would help here. This is a bad implementation, and what your question seems to imply is the case:
// Consumer
var court = new CourtOfJustice();
var defendant = new Person();
var shouldBeTriedAsAMinor = true;
court.Prosecute(defendant, shouldBeTriedAsAMinor);
// Implementation
class CourtOfJustice
{
public void Prosecute(Person defendant, bool shouldBeTriedAsAMinor)
{
if(shouldBeTriedAsAMinor)
ProsecuteMinor(defendant);
else
Prosecute(defendant);
}
private void Prosecute(Person defendant)
{
// ...
}
private void ProsecuteMinor(Person defendant)
{
// ...
}
}
The following is a good implementation. Notice how it barely differs from the bad one, and still fits with your question's description:
// Consumer
var court = new CourtOfJustice();
var defendant = new Person();
var defendantIsAMinor = true;
court.Prosecute(defendant, defendantIsAMinor);
// Implementation
class CourtOfJustice
{
public void Prosecute(Person defendant, bool defendantIsAMinor)
{
if(defendantIsAMinor)
ProsecuteMinor(defendant);
else
Prosecute(defendant);
}
private void Prosecute(Person defendant)
{
// ...
}
private void ProsecuteMinor(Person defendant)
{
// ...
}
}
The only thing that has changed is the name of the method parameter, but you also have to consider that the context has changed, i.e. the implication that the consumer somehow decides how the defendant should be tried. They shouldn't. All the consumer is doing here is providing information (is the defendant a minor?). It is the court which decides how to try a defendant.
As far as the consumer is concerned, they don't even know what difference it makes whether the defendant is a minor or not. Maybe it doesn't change how they are prosecuted, it only changes what prison they get sent to. Or whether the court documents will be sent to a parent instead of them. The consumer does not know the impact of the value they pass. They only know that the court has asked them to provide this information.
To summarize, the only issue with flag arguments is if the consumer knows to use the input data to steer the implementation. As long as the consumer merely provides input, without any expectation of knowing how this will impact the implementation, it's not a bad approach.
Strategies
You may think that because your enum specifically lists operations, which inherently imply methods, that you are therefore unavoidably violating this principle. Well, sort of.
The issue here is that the choice of what operation to execute is being made by the consumer. This in turn means that the choice of calling the right method should not be implemented by the implementation, because it is not the one making the choice.
Whoever makes the choice, also has to reference the choice they made.
This is what is called the strategy pattern. Instead of passing an enum value, you pass an object which is capable of performing the needed operation. Therefore, the implementation receives a "black box", does not know what operation this will perform, but the implementation can still call it.
Note: this really only makes sense when your called method is doing more than just calling the submethod. But I will get to that afterwards.
For example:
// Consumer
process(data, new Confabulator());
// Implementation
public void process(Data data, IOperation operation)
{
operation.Execute(data);
// some more logic related to the "process" method itself
}
public interface IOperation
{
void Execute(Data data);
}
public class Confabulator : IOperation
{
public void Execute(Data data)
{
// ...
}
}
public class Reticulator : IOperation
{
public void Execute(Data data)
{
// ...
}
}
Notice how it is the consumer who chooses what operation to perform, and also chooses the right strategy to perform this chosen operation. The implementation merely executes whatever operation the consumer passed into it.
However, in your code there is no such some more logic related to the "process" method itself
. This means that the process
method does nothing except call the passed operation, and therefore it has become redundant. It can be wholly removed:
// Consumer
IOperation operation = new Confabulator();
operation.Execute(data);
// Implementation
public interface IOperation
{
void Execute(Data data);
}
public class Confabulator : IOperation
{
public void Execute(Data data)
{
// ...
}
}
public class Reticulator : IOperation
{
public void Execute(Data data)
{
// ...
}
}
If the process
method had contained more logic, the previous snippet would be useful. But when it doesn't, the last snippet already does what you need.
@Mapping("/operation/confabulation")
, etc.).