2

So I have an Interface MyInterface<T> which has a method String save(T obj) The implementing class has this definition Animal<T> implements MyInterface<T>. My Main function looks like this:

MyInterface<Cow> animal = new Animal<Cow>();
Cow cow = new Cow();
cow.setName("Rose");
animal.save(cow);

So far so good, animal.save() can only save Cows which is type-safe.

Now the problem. In my Animal class I have the save method, but before I save I want to know that my Cow has a name. The Cow class has a getName() method.

Save method:

@Override
public void save(T obj)

What would be an apropriate way to go from T obj to a Cow-object so I can use the member method getName()?

My idea is:

@Override
public void save(T obj)
{
        Object unknown = obj.getClass().newInstance();
        if (unknown instanceof Cow)
        {
            String name = ((Cow) unknown).getName();
        }
}

Is this "ok" java? I guess I could use reflection and search for all methods related to the unknown object as well..

Update As long as I define MyInterface<Cow> animal = new Animal<Cow>(); Like this I'm safe, but the problem apperas when it looks like this:

MyInterface<Object> animal = new Animal<Object>();

Then there is no getName() method....

Baked Inhalf
  • 3,375
  • 1
  • 31
  • 45
  • 2
    Using `instanceof` to check the type of an object often means your code has a smell. The whole point of coding to an interface is so that you can call `getName()` on _any_ matching generic object, without worrying that the object has the `getName()` method. – Tim Biegeleisen Dec 21 '17 at 10:31
  • is your Cow class a subclass of Animal? – AdamPillingTech Dec 21 '17 at 10:32
  • It is "ok", but hard to maintain. Imagine you now implement a `Dog`, you may want to check its `goodBoiScore()`. For a `Cat`, you may want to check its `meow()`,... For each of this new classes, you would have to create a new `if (... instanceof ...)`. This seems like an [XY-problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem) to me. Could you explain what you are actually trying to do with e.g. the `cow`'s name? – Turing85 Dec 21 '17 at 10:33
  • Yes I don't like the instanceof, but feels scary to leave it out. I cant pass an object which isn't a Cow to the save method in the first place... – Baked Inhalf Dec 21 '17 at 10:33
  • 1
    My question is if you specifically _only_ want this to happen for Cow objects, or for all classes that implement this interface? Could you not write a (virtual) getName() method in the interface of MyInterface or Animal so you can call this method without having to worry that the object has to be a Cow? – Christophe Loeys Dec 21 '17 at 10:35
  • @Turing85 I was looking at the ArrayList source code. It works similar, but as Tim said, maybe the instanceof is not needed – Baked Inhalf Dec 21 '17 at 10:35
  • @BakedInhalf could you point out which part of `ArrayList`'s source code you mean? There are some casts to `T` within `ArrayList`, but as far as I know, they are all type-safe, even if the backing Array is an `Object[]`. As far as I know, there are no `instanceof`-instructions within ArrayList (JDK 1.8, have not checked JDK 1.9). – Turing85 Dec 21 '17 at 10:37
  • @ChristopheLoeys That is one good idé. Will try it out – Baked Inhalf Dec 21 '17 at 10:43
  • @Turing85 I figured out I don't need the cast as long as I define in angle brackets what class i'm working with. The problem arises when the definition looks like this: ´MyInterface animal = new Animal();´ – Baked Inhalf Dec 21 '17 at 10:44
  • 1
    If you use `Object` as generic parameter, you pretty much give up almost all type security =) to avoid this, we can use `extends` and `super` to restrict generic types. You may want to take a look at the [PECS mnemonic](https://stackoverflow.com/questions/2723397/what-is-pecs-producer-extends-consumer-super). – Turing85 Dec 21 '17 at 10:47
  • @Turing85 Whoha that was some heavy complex stuff =) I guess I can read that in my spare time just after Santa =) – Baked Inhalf Dec 21 '17 at 10:52

4 Answers4

4

If you assume that all animals have a name then create an interface.

static interface HasName {
    String getName();
}

All objects having a name should then implements the interface

class Cow implements HasName{
    String name;

    Cow(String name){
        this.name = name;
    }

    @Override
    public String getName() {
        return name;
    }
}

Then modifying Annimal to allow all objects implementing the interface you will be able to get their names:

class Annimal<T extends HasName> {
    void save(T obj) {
        ...
        String name =  obj.getName();
    }
}

Even if the use of instanceof works, like in your post, it's a practice to avoid, because if one day you want to implement a new type of animal, let's say Horse, you will have to modify the implementation of your save method to add the if (unknown instanceof Horse) condition.

Guillaume Barré
  • 4,168
  • 2
  • 27
  • 50
3

instanceof should be avoided wherever possible, and so should reflection.

I would add a separate interface which indicates whether something is in a saveable state. Your Animal class would then need to change it's generic type parameter to only allow saveable items.

interface Saveable
{
    boolean canSave();
}

public class Cow implements Saveable
{
    //...

    @Override
    boolean canSave()
    {
        return getName() != null && !getName().isEmpty();
    }
}

public class Animal<T extends Saveable> implements MyInterface<T>
{
    @Override
    public void save(T obj)
    {
        if (obj.canSave())
        {
            //whatever
        }
    }
}
Michael
  • 41,989
  • 11
  • 82
  • 128
  • That I didn't think of, really nice! I will try your approach. Didn't know a class could "extend" an interface on it's generic type! – Baked Inhalf Dec 21 '17 at 10:48
2

No, this is "not OK" Java. First because you want to avoid downcasts, but mainly for your idea to use reflection like this.

If that getName() functionality is really an essential part of your model, then consider placing it in its own interface for example.

Meaning: if at all, you should be doing something like

@Override
public void save(T obj) {
  if (obj instanceof NamedThingy) { 
    then cast ...
GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • I guess I don't need the cast... it's not even possible to pass an object of Object() to the save method. As long as I define in angle-brackets what type I'm working with . Just saw that now :) – Baked Inhalf Dec 21 '17 at 10:38
  • "not OK" - very subjective. Downcasting does happen from time to time. Sometimes you're stuck with code that isnt yours and you cant change, for example. – vikingsteve Dec 21 '17 at 10:43
  • I was mainly concerned about using reflection - updated my answer accordingly. – GhostCat Dec 21 '17 at 11:31
1

All the answers are great and can be used without a doubt. But I still have something to suggest. While the others most time talked about a new interface and then invoke a method like getName() or canSave (which does work like a charm), I prefer using a custom tester, like a Predicate<T>.

I assume your MyInterface is like a service which saves an entity to a database or a similar data structure. Then you could adapt your class Animal like the following:

public class Animal<T> implements MyInterface<T>{
     private final Predicate<T> canSave;

     public Animal(Predicate<T> canSave){
         this.canSave = canSave;
     }

     @Override
     public void save(T obj){
         if(canSave.test(obj)){
              // your save logic
         }
     }
}

That way you define per instance of Animal a behaviour when that entity can be saved:

Animal<Cow> animal = new Animal<>(c -> c.getName() != null); // save only when name not null
Cow cow = new Cow();

animal.save(cow); // doesn't save because name is null

cow.setName("Rose");
animal.save(cow); // saves because name is set

animal = new Animal<>(c -> true); // save always

cow.setName(null);
animal.save(cow); // saves because it can always
Lino
  • 19,604
  • 6
  • 47
  • 65
  • 1
    I felt the `new interface` was the most convenient solution, but this was cool. – Baked Inhalf Dec 21 '17 at 12:20
  • @BakedInhalf it really depends on your usecase. Because when you only have to check if the name is set, for every animal, then `new interface` is the best approach, because it's not overengineered. If you have different conditions though, then my answer would probably work the best – Lino Dec 21 '17 at 12:32