An app I’m working on is designed with MVC. The components often interacts with each other by passing int constants, which then have to be interpreted with if statements. I’m wondering if this is considered bad OO design. (Not specifically with MVC, but in general).
For example:
When the user clicks a specific button, the controller is told by the view controller.somethingHappened(Controller.SOME_INT_VALUE);
The view uses a constant int value in the controller as a message to tell what happened. The controller then has to have a lot of if statements to interpret the message from the view. E.g.:
public void somethingHappened(int message){
if(message == AN_INT_VALUE) doSomething();
if(message == ANOTHER_INT_VALUE) doSomeThingElse();
if(message == A_DIFFERENT_INT_VALUE) doAnotherThing();
}
As a result the controller passes another int value into the model, as a message to do something. I.e. model.doSomething(Model.SOME_INT_VALUE);.
Which forces to model to also have a bunch of if statements:
public void doSomething(int message){
if(message == AN_INT_VALUE) doSomething();
if(message == ANOTHER_INT_VALUE) doSomeThingElse();
if(message == A_DIFFERENT_INT_VALUE) doAnotherThing();
}
This results in both the controller and model having a lot of ifs to interpret the messages the receive.
Is this considered ugly and/or bad design? Should I try to pass actual objects into methods, or make more specific method calls – instead of passing constants that have to be interpreted with a bunch of ifs? Or is this practice of using a lot of constants as messages considered reasonable?
If it is considered bad design, how can I avoid this?
(Please note: For the purpose of this question I’m also referring to enums).
6
Is this considered ugly and/or bad design?
Yes.
Should I […] make more specific method calls – instead of passing constants that have to be interpreted with a bunch of ifs?
Yes, that’s exactly what you should do. Unless there is some reason not to. Given that you’ve already thought of this much cleaner option yourself, I suspect that there might be, but it’s not discernible from your question.
Update: If the called methods would have duplicate functionality, the object oriented solution is to put the functionality that differs into separate classes that all implement the same interface, and call that from within the function, using polymorphism instead of explicit branching.
So, using the information from the comment, you’d call
controller.applyStyle(new BoldStyle());
or
controller.applyStyle(new UnderlineStyle());
where BoldStyle and UnderlineStyle both are classes that implement a Style interface (or inherit from the same superclass) which contains one or more methods that do the different things and which are called by the applyStyle() method.
9
There are several choices here, in increasing order of preference (most preferable last):
-
Bits (flags) in an integer. Can be confusing, but allows packing a lot of them into a single int. So
DO_THIS = 1
DO_THAT = 2
DO_ANOTHER = 4
DO_ZING = 5perform(DO_THIS | DO_THAT)
if (command && DO_THIS{ doThis(); }
…etc -
Integer commands
DO_THIS = 1
DO_THAT = 2
DO_ANOTHER = 3perform(DO_THIS, DO_THAT)
for (command in commands):
if (command == DO_THIS) { doThis(); } -
I favor strings over ints, just because they’re more readable
DO_THIS = “this”
DO_THAT = “that”
… -
“Typesafe” constants (in case you don’t have enums around)
DO_THIS = new Object();
DO_THAT = new Object();perform(DO_THIS, DO_THAT);
for (command in commands)
if (command == DO_THIS) { doThis(); } -
Enum is a formalization of “Typesafe constants” so use that if you have them
…
Generally, though, these schemes are useful when the calls have to be serialized & transmitted between processes. If you’re just making calls from one section of code into another, you can just identify what you want done with the function names. Writing some sort of “router” function that makes a call to lower-level function replicates what the language does for you, so you might think harder about whether you need that.
…
That is, I try to keep it so that 90% of my code looks like it came straight out of “Learn Java in 30 days” (or Python, or whatever). That is, the code should just use the language in a really basic way almost all the time. There should be a pretty good reason why you can’t just do it the basic way before you move to a broader generalization.
3