return values of matsim getters

[kainagel]

I put the following also here: https://docs.google.com/document/d/1CSvPcMKrPy-0j_RBvMEyCyqqDq_JH1snFkGUlTrLIKM/edit?usp=sharing because that seems better as a discussion format.

Prompted by https://github.com/matsim-org/matsim-libs/pull/2669#issuecomment-1617585066 .

MATSim andheres to the convention that class variables are not directly accessible, but via getters and setters. Evidently, there is an issue of what to return if a value/an object is not there. We identify the following options, all of which can be found in MATSim code:

1. return “null”
2. return NaN or similar
3. throw an exception
4. return an instance of Optional

Some comments to these options

1. This is the Java standard as long as objects are concerned.
2. If the item under consideration is a primitive type, returning null is not possible. One also does not want to convert it to an object for computational speed reasons. We often return NaN, which however has the disadvantage that “if ( aaa == Double.NaN )” does not behave as desired since NaN compares false with everything. In consequence, one needs to use “if ( Double.isNaN( aaa )”. This is prone to errors.
3. Throwing an exception means that user code needs to use try/catch syntax to hedge against that, which is ugly and makes code more opaque.
4. Some people have started using Optional in MATSim, but Intellij warns against the (over?)use of Optional. I have not looked into the reasons. In any case, optional returns an object where it may have been a primtive type before, making some numerical computationals much slower. There was a very small discussion related to what something like VehicleUtils.hasVehicleId( … ) should return, i.e. a wrapper about what is attached (or not) to an object as Attribute. “marecabo” eventually writes here:

@kainagel Thanks for your comment. I agree, that making such private member public should not be an option as everyone will write own helpers that might do things slightly different. Then, to throw my two cents into the discussion: ;) One consideration would be, that this helper methods are as efficient as possible regarding computational and memory allocation demands. This is especially relevant for large populations when these methods might be called, say, ten million times. If returning an exception, it should be a custom exception class, and not some generic RunTimeException, where it is required to compare parts of the exception message (with a hard-coded substring of it) to identify what happened - which introduces another antipattern.

This seems to imply that the above options 2. and 3 would only be acceptable in outer loops since otherwise they might become too slow. I cannot say if this is the case or not. Java Collections adresses something similar for Queue by offering both:

• queue.peek(), queue.poll() return null if the queue is empty.
• queue.remote(), queue.element() throw an exception if the queue is empty. Since I don’t think that we will reach agreement on what would be a good matsim standard, an option would be to allow for both.

→ What do we think?

If so, it would also be good to agree on a standard. It seems to me (personally) that most getters currently return null. So the pragmatic way would be to leave it at that, but to add, where people want this, a “getOrFail…()” and/or a getAsOptional. And maybe “getOrNull” for cases where the standard get has already been converted to returning Optional.

→ What do we think?

[steffenaxer]

Some thoughts from my side. Creating an exception and catch it is cost intensive as new exception objects needs to be instantiated. The same thing is the optional approach, isn’t it? In terms of performance, it might be useful to avoid such patterns. I already use at some places getOrNull. Thus, I would vote for this approach.