1

Context: I believe that object creation and management in Java has a cost that we should bear in mind while programming. However, I don't know how big that cost is. Hence my question:

I have multiple functions that share the same arguments:

  • detectCollision(ArrayList<Mobile>, ArrayList<Inert>, double dt)
  • updatePositions(ArrayList<Mobile>, double dt)
  • etc.

As I see it, there are two ways to organize them (see code below):

  1. define (possibly static, but not necessarily) methods and forward the arguments for each call
  2. create a temporary object with private member variables and remove argument list.

Note that the Mover object has no private internal state and is just a bunch of algorithms that use the arguments ArrayList<Mobile>, ArrayList<Inert>, double dt.

Question: Which approach is the prefered one ? Does it have a cost ? Is there a more standard alternative ?

Here is a snippet illustrating the first point:

public class Mover{
    public static void updatePositions(ArrayList<Mobile>, double dt){...}
    /* remove the static keyword if you need polymorphism, it doesn't change the question */

    public static Collisions detectCollision(ArrayList<Mobile>, ArrayList<Inert>, double dt){...}
    //etc.
}

Here is a snippet illustrating the second point:

public class Mover{
    public Mover(ArrayList<Mobile>, ArrayList<Inert>, double dt){...}
    public void updatePositions(){...}
    public Collisions detectCollision(){...}
    //etc.

    private ArrayList<Mobile> mobiles;
    private ArrayList<Inert> inerts;
    //etc.
}
Julien__
  • 1,962
  • 1
  • 15
  • 25
  • 3
    Don't worry about performance at this stage. Worry about the readability, maintainability of your code, and how well the code's model of reality matches that reality. Worry about performance only when you can demonstrate that there is a performance problem. – Jim Garrison Mar 16 '16 at 17:55
  • @JimGarrison, "Premature optimization is the root of all evil". Thank you for you feedback. Which one do you find more readable ? – Julien__ Mar 16 '16 at 17:56
  • Design is all about trade offs. Can you please explain Mover Object and the context in which it is used? – Salman Virk Mar 16 '16 at 17:57
  • @svirk: the `Mover` object has no private internal state and is just a bunch of algorithms that use the arguments `ArrayList`, `ArrayList`, `double dt`. – Julien__ Mar 16 '16 at 18:03
  • 1
    @Julien__ In this case, Mover is not even an object ( from designer's point of view ). Mover is a Module that holds similar functions. If you agree with me, then the first design is better. I would add a note in the comment of Mover class that it is a module and not an object. – Salman Virk Mar 16 '16 at 18:06
  • 2
    Off topic, but while we're at it (regarding the design) : You should basically **never** expect an `ArrayList` as a parameter. Always use `List` instead (and likely even a `List extends Mobile>`). Also see http://stackoverflow.com/questions/383947/what-does-it-mean-to-program-to-an-interface – Marco13 Mar 16 '16 at 18:50

3 Answers3

2

I'd recommend you to go with the second variant. Besides the good readability it will also allow you to extend the class later (see SOLID -> open / closed principle). In general, I'd never create such utility classes, as it is not OOP (OOP Alternative to Utility Classes).

flogy
  • 906
  • 4
  • 16
  • How would you turn those functions (moving objects, collsion detection) using such a method ? – Julien__ Mar 16 '16 at 18:23
  • Depends on what you are trying to acheive. It is not clear what your Mover actually is responsible for? Is it kind of a controlling mechanism that calculates the movements for some entities (-> Mobile) and then sets their positions? What are the Mobile and Inert? What does dt represent? Maybe some implementation code would help, too. – flogy Mar 16 '16 at 18:35
  • you can consider the Mobiles to be Sphere and Inerts to be Cubes. Update position computes (pos += speed * dt) and detectCollision solves an equation to predict the date of the next collision (infinity if none) – Julien__ Mar 18 '16 at 17:00
1

While I think that static utility methods are not necessarily a Bad Thing™, you should be aware of the semantics of these methods. One of the most important points here is that static means that a method may not take part in any form of polymorphism. That is: It may not be overridden.

So regarding the design, the second option offers a greater degree of flexibility.


A side note: Even if you choose the second approach, you might eventually dispatch to a (non-public) static method internally:

class Mover {
    private final List<? extends Mobile> mobiles;

    public void updatePositions(){
        MyStaticUtilityMethods.updatePositions(this.mobiles);
    }
}

Note that I'm not generally recommending this, but pointing it out as one option that may be reasonable in many cases.


It might be off-topic, but there is another degree of freedom for the design here. You could (and at least should consider to) go one step further: As far as one can guess (!) from the method names, you might consider having interfaces PositionUpdater and a CollisionDetector. Then you could store instances of classes implementing these interfaces inside your Mover class, and dispatch the actual call to these. This way, you can easily combine the different aspects of what comprises a "Mover".

(I know, this does not answer the actual question. In fact, it just "defers" it to whether the PositionUpdater should receive the data as arguments, or receive them at construction time...)

You could then assign instances of different implementations of the PositionUpdater interface to a Mover instance. For example, you could have concrete classes called LinearMovementPositionUpdater and RandomWalkPositionUpdater. Passing instances of these classes (which are both implementing the PositionUpdater interface) to the Mover allows you to change one aspect of the implementation of a Mover - basically, without touching any code! (You could even change this at runtime!).

This way, the responsibilities for

  • updating the positions
  • detecting the collisions

are clearly encapsulated, in view of several of the SOLID principles that already have been mentioned in another answer.

But again: This is just a hint. Judging whether this approach is actually sensible and applicable in one particular case is what software engineers get all the $$$ for.

Marco13
  • 53,703
  • 9
  • 80
  • 159
  • Static utility methods are incredibly difficult to mock, and so should be avoided like the plague. They are, entirely and totally, a bad thing. – Software Engineer Mar 17 '16 at 12:55
  • 1
    @EngineerDollery I had extensive (but inconclusive) discussions about this at other places. For me, a static utility method is a building block with fixed functionality. If you have 20 lines of code with a clear, functional *arguments->result* semantics that is required in 5 unrelated classes, then there is no point in replicating this code. Whether or not this code is "inlined" in a method or implemented via a single static method call may not even be visible in the public interface (which is about to be tested and possibly mocked) at all. But likely, we cannot discuss all this here. – Marco13 Mar 17 '16 at 13:45
  • I fall into the 'utility classes are evil' camp. eg: It's common to load files from the classpath via a static method, but if I'm testing I don't want to load a file I just want the resulting data-structure to be well-defined without having to go through the hassle of creating a file that will be parsed in an appropriate way to produce the resulting structure I'm after. If the loadFile method is static I have little choice. And that's simple things -- the OP's problem is much harder. – Software Engineer Mar 17 '16 at 13:59
  • @EngineerDollery Again, design topics are difficult (in fact, I found it nearly surpising that this question was not closed as "opinion based"). Of course, what you described is indeed evil. But consider (whatever) a *purely functional (!)* method like `Point computeIntersection(Line a, Line b)` that is needed (as an overridable/mockable instance (!) method) in 5 otherwise unrelated classes. There's no point in implementing this 5 times, instead of just doing `...{ return MyInternalStaticMethods.computeIntersection(a,b); }`. Static utility methods *can* be fine for "non-behavioral" code parts – Marco13 Mar 17 '16 at 15:49
  • Hi @Marco13, thank your for pointing the polymorphism issue (+1). It doesn't change the question, however, since you can write the same thing and simply remove the static keyword. As for the SOLID principles, I totally agree with them. In this particular example, you can't decouple collision detection from moving strategy because you need strong assumptions on the way objects move in order to predict a precise collision time. – Julien__ Mar 18 '16 at 16:57
  • @Julien__ In many "simulation-like" scenarios, the moving objects store e.g. the position and the velocity. This allows you to derive *where* the object will move in the next step, and the collision detection then often means *adjusting* the velocity so that no collision will happen. But sure, this is one of the points that I referred to: It's hard to know whether this is applicable in your particular case. – Marco13 Mar 18 '16 at 19:34
1

The problem you're having here is mostly due to having an orchestrated model (and which probably has an anaemic domain model to accompany it).

If instead you use an event driven approach these methods would simply disappear and be replaced by event handlers each of which would respond to the appropriate events independently of the other, taking whatever information they need from the initial event.

This means that you wouldn't have to deal with passing parameters or working out which parameters to pass -- each 'handler (method)' would know what it needs and would take it, rather than having to have an external orchestrator understand what data is needed and pass it in.

The orchestrator model breaks encapsulation by having it 'know' about the information needs of the components it is orchestrating.

Now, this isn't a java specific problem. It applies to most modern object-oriented languages. It doesn't matter if you're doing java, c-sharp, or c++. It's a general pattern.

To break away from this thinking, read about DDD (Domain Driven Design) and Event Sourcing.

Software Engineer
  • 15,457
  • 7
  • 74
  • 102