0

Our library contains two methods: doXForAll and doXForEach:

default List<Object> doXForAll(Object input1) {
    return Collections.singletonList(doXForEach(input1));
}

default Object doXForEach(Object input1) {
    return doX(input1); // Some kind of default behaviour over the input
}

We encourage clients to override these default methods with their own implementations. We need to be cautious not to make any changes that would break existing functionality.

The problem:

We want to add support for an extra parameter Object input2 in both of the above methods. How would you recommend going about this without requiring our clients to update their implementations?

Our current solution:

default List<Object> doXForAll(Object input1, Object input2) {
    return doXForAll(input1);
}

The problems with this are:

  • We're missing a doXForEach method with the additional parameter
  • We are calling methods we've just deprecated (which feels wrong)
Community
  • 1
  • 1
Arachin
  • 97
  • 2
  • 13
  • 1
    Can you not just create an overload for these methods? – AleksW Apr 23 '19 at 15:23
  • Yes, but if it became the entry point, existing client overrides would no longer be called, thus breaking existing functionality – Arachin Apr 23 '19 at 15:24
  • it will become the entry point only if the new parameter is provided.... if the existing clients want their code to be the entry point too using the new parameter, they would have to override the overloads too. – lealceldeiro Apr 23 '19 at 15:26
  • 4
    Why would this be the case? If they have an override of `doXForAll(Object input1)`, you creating a `doXForAll(Object input1, Object input2)` overload should not cause any issues (@Arachin) – AleksW Apr 23 '19 at 15:27
  • Would it be possible to call the "old" method with just the first parameter, i.e. something like `doXForAll(input1, input2) { /*old method*/ doXForAll(input1); /*use input2 here*/}`? Would it be ok to ignore that additional parameter, i.e. if you could detect the one-parameter version has been overridden you'd just call that and be done? – Thomas Apr 23 '19 at 15:28
  • 1
    https://stackoverflow.com/questions/997482/does-java-support-default-parameter-values – Luis Dipak Apr 23 '19 at 15:38
  • I've updated the question to cover the common suggestions here (we tried the same) @AleksW – Arachin Apr 23 '19 at 15:43

2 Answers2

1

For the We're missing a doXForEach method with the additional parameter point, what's stopping you from creating this?

And as for We are calling methods we've just deprecated (which feels wrong), you shouldn't be calling the deprecated methods.

E.g. just create something like this in addition to what you already have:

default List<Object> doXForAll(Object input1, Object input2) {
    return Collections.singletonList(doXForEach(input1, input2));
}

default Object doXForEach(Object input1, Object input2) {
    return doX(input1, input2);
}
AleksW
  • 703
  • 3
  • 12
  • This would give us two entry points: One for `doXForAll(input1)` and one for `doXForAll(input1, input2)`. If we make use of the latter then the overriding methods our clients have created will no longer be called, breaking existing behaviour. – Arachin Apr 23 '19 at 16:35
1

You are looking at the prospect of supporting as many as 16 possible scenarios. The following four cases can each occur independently, so 2^4=16.

  1. The client does/doesn't override doXForAll(Object input1)
  2. The client does/doesn't override doXForEach(Object input1)
  3. The client does/doesn't override doXForAll(Object input1, Object input2)
  4. The client does/doesn't override doXForEach(Object input1, Object input2)

If you implement your current solution, then you will experience the following unfortunate result:

  • if the client overrides both doXForAll(Object input1) and doXForAll(Object input1, Object input2)
  • and you call doXForAll(Object input1, Object input2) via your interface
  • then the client's doXForAll(Object input1) will be called
  • but the client would probably expect doXForAll(Object input1, Object input2) to be called instead.

To prevent such surprises, it would be better to treat the new methods as different from the old ones and have them be overridden separately.

DodgyCodeException
  • 5,963
  • 3
  • 21
  • 42
  • Agreed, and this is the solution we went for. I'll pop it up here once it's been through review (to make sure I haven't missed anything). – Arachin Apr 26 '19 at 13:21