3

I'm not quite sure how to word this question, but hopefully the example will make it a bit more clear. I'm trying to figure out the best way to have one of the implemented abstract methods not be called (do nothing) and I'm curious if my current approach is at all somewhat right.

abstract class Vehicle {
  void doSomething() {
    if (this.getClass() instanceof Chevy) {
      operateOnCar();
    }
  }
  abstract void operateOnCar();
}

class Chevy extends Vehicle {
  @Override
  void operateOnCar() {
    print("Doing something to a car")
  }
}

class HarleyDavidson extends Vehicle {
  @Override
  void operateOnCar() {
    throw Exception("This isn't a car")
  }
}

The other approach I can think of is to have the HarleyDavidson class implement operateOnCar()but do absolutely nothing - i.e. an empty method body. Is that potentially better? Maybe neither of these are viable examples and I should reconsider my design. Thanks!

Edit: tried to make my example a bit more clear

joshft91
  • 1,755
  • 7
  • 38
  • 53
  • 2
    `isCar` sounds like it should return a `boolean` to me. In which case it would perfectly valid and meaningful for a subclass to implement it as `return false;`. – CollinD Aug 22 '18 at 19:18
  • 3
    `isCar` should return a boolean. The default implementation should return false, `Car`s should return `true`, and `Chevy` should most likely not be a class, as the brand is a characteristic of a vehicle. Your `print` is doing it wrong. – Dave Newton Aug 22 '18 at 19:18
  • Yeah, sorry, I'll update my example. It's more meant towards void methods I suppose... in the Harley class I don't want `isCar` called at all. I'll udpate it here – joshft91 Aug 22 '18 at 19:20
  • 2
    @joshft91 Why would you put a car-specific method in `Vehicle`? – Dave Newton Aug 22 '18 at 19:22
  • @DaveNewton good question. So in my case I have 3 total subclasses. 2 use "car" method and the other doesn't use it entirely. I suppose I could pull the car method out and put that in the overridden `doSomething` method in the concrete class? – joshft91 Aug 22 '18 at 19:24
  • 1
    `this.getClass() instanceof Chevy` should result in a compile time error, since `this.getClass()` returns a `Class` object which is not on the same class hierarchy as `Chevy`. In simple words, write, `this instanceof Chevy` instead. :) – Kröw Aug 22 '18 at 20:18

4 Answers4

2

I'm trying to figure out the best way to have one of the implemented abstract methods not be called (do nothing)

Asserting that a method should not be called is totally different from asserting that it should do nothing. It is furthermore wrongheaded to define a method on the superclass, regardless of abstractness, that is not ok to call on any instance of any subclass. Thus, some variation on the "do nothing" alternative is a much better choice.

And what's so hard about a method doing nothing when it does not need even to provide a return value? This is a method that does nothing:

void isCar() {
    // empty
}

I should also observe at this point a method named isCar would be expected by most Java programmers to return a boolean indicating whether the object on which it is invoked is a "car", whatever that means in context. It would come as a surprise that such a method is declared not to return anything, and perhaps an even bigger surprise that it writes to System.out.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Thanks for you response - yeah I rushed my example and didn't do a good job with naming. I updated my example to try and make it a bit more clear... I would rather go with an empty method on the concrete class than check for types of vehicles, I just wasn't sure if that was "bad practice" – joshft91 Aug 22 '18 at 19:26
  • @joshft91, an empty method on the concrete class is not inherently bad practice. You could also consider an empty concrete method on the abstract superclass, so that only the subclasses that are, in fact, cars need to override it. – John Bollinger Aug 22 '18 at 19:30
  • Interesting, that's a good suggestion. So I could have an empty concrete method in the abstract class that does nothing and then it's up to developer knowledge to know to implement the overriding method in the concrete class that does what I want it to? – joshft91 Aug 22 '18 at 19:36
  • 1
    Yes, @joshft91, that's the alternative I was describing. – John Bollinger Aug 22 '18 at 19:42
2

You're blurring the responsibilities of your abstract class and its concrete implementations. This is evidenced by your doSomething method.

void doSomething() {
    if (this.getClass() instanceof Chevy) {
      operateOnCar();
    }
}

Your abstract class shouldn't care what instance it is; only the Chevy class needs to do something with this.

This may be why you're getting mixed up with the operateOnCar method. A Car is a Vehicle, but not all Vehicles are Cars. You could have trucks, vans, locomotives, boats, planes...all of which are vehicles in their own right, but definitely wouldn't support an operateOnCar method.

It may be as simple as renaming your method. You can definitely operate or fix a vehicle. You just want to keep that as agnostic as possible at the higher levels of the inheritance chain.

Makoto
  • 104,088
  • 27
  • 192
  • 230
  • That's what I was thinking as well. I definitely didn't do a good job naming my example code, but I guess I wanted to see if there was a bad or wrong reason to have a concrete class's implementation of an abstract method do nothing. It certainly seems like a better alternative than checking the types of the classes in the abstract one – joshft91 Aug 22 '18 at 19:29
  • 2
    @joshft91: Yes, it's bad for a parent class to define child-specific behaviors. This breaks encapsulation and blurs responsibilities across class boundaries. – Makoto Aug 22 '18 at 19:48
2

If you make a class, than that class (including its derived classes) should only have properties/methods related to that class. If a property/method does not fit, it shouldn't be in a class.

So how you can put OperateOnCar somewhere: with the strategy pattern, which is a common design pattern.

Than you make an interface OperatableOnVehicle, with an operation OperateOnVehicle. For vehicle it will be implemented, for a HarleyDavidson it is not implemented.

Try to avoid to use the word 'car'.

There are many examples to be found on internet about the strategy pattern, so google for more information about it's background.

Michel Keijzers
  • 15,025
  • 28
  • 93
  • 119
  • `IOperatable` is a name for interface I really don't like to see anywhere in the Java world. Is this inherited from C#? – Nikolas Charalambidis Aug 22 '18 at 19:28
  • @Nikolas I changed it to IOperateOnVehicle, although repeating a class name in a property/method name is also not a good decision, but it seems IOperatable has a special meaning in Java. – Michel Keijzers Aug 22 '18 at 19:33
  • 1
    Well, putting the big I into the interface name isn't good practice in Java. – GhostCat Aug 22 '18 at 20:16
  • @Ghostcat Ok (really? I have a Java patterns book that always uses it if I remember right)...but I will remove it here. – Michel Keijzers Aug 22 '18 at 20:23
  • 1
    I *think* it was proposed and used at some point, but not widely used any more, at least in Java. See https://stackoverflow.com/questions/541912/interface-naming-in-Java for example. – GhostCat Aug 22 '18 at 20:27
  • 1
    Java naming conventions were heavily influenced by Sun in the early(ish) days, and Sun (and Oracle after it) never advocated for distinguishing interfaces from classes via their names. By avoiding that convention in naming any of the many the interfaces in the Java standard library and by omitting any mention of that naming practice in the Java Tutorial, Sun pretty firmly quashed most third-party use of that convention, too. Personally, I've always associated the convention with Microsoft. – John Bollinger Aug 22 '18 at 21:10
1

I think if object calling this method throws the following exception:

throw Exception("This method should only be called if the car is a chevy")

There would be better to print out that it's not a car. As the name of the method suggests, it should return boolean

  • Is Chevy a car? Yes it is - return true and inform about the result
  • Is HarleyDavidson a car? No, it is not - return false and inform as well
  • is for exampleBicycle a car? No, definitely not. Is it a vehicle? Yes. Is legitime to ask it whether it's a car? Why not?

By the way, have you considered the following hierarchy?

  • Vehicle <- Car <- Chevy
  • Vehicle <- (Motocycle ) <- HarleyDavidson
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
  • I based my answer on common sense. Give me feedback - I am curious other thoughts. – Nikolas Charalambidis Aug 22 '18 at 19:26
  • Yeah, I'm realizing that I chose a terrible naming convention to try and represent the problem I'm trying to solve... this post has so far been a great example of that. *facepalm – joshft91 Aug 22 '18 at 19:31
  • Although you have changed the naming convention, the problem persists. Follow the simple logic I have introduced in my answer and prefer to create more classes over making the existing ones even more complicated. – Nikolas Charalambidis Aug 22 '18 at 19:34
  • I'd consider using a hierarchy like the one that Nikolas gave, if you're up to changing yours. :/ – Kröw Aug 22 '18 at 20:21