0

I refactored a class out in to many that implement an interface instead. It works fine, however I'd like to be able to use the code without having to infer or suppress warnings everywhere.

//Item is a raw type. References to generic type Item<T> should be parameterized.
public interface Item<T extends Item>
{
    // Item is a raw type. References to generic type Item<T> should be parameterized.
    public static final Item DEFAULT_ITEM = new FooItem(Color.RED);

    // Because using "something = new Item(Default_Item)" won't work.
    public T Copy();

    public void Foo();
    public void Bar();
}

public class RepairMan
{
    // Item is a raw type. References to generic type Item<T> should be parameterized.
    Item itemNeedingRepairs;

    // Item is a raw type. References to generic type Item<T> should be parameterized.
    public RepairMan(final Item item)
    {
        itemNeedingRepairs = item.Copy();
    }       
}

I would like to be able to use Item as a regular non-interface version in the majority of code. There's too many places to add ignore warnings, and Item everywhere doesn't seem appealing either.

Some notes:

  1. Item doesn't make sense being a class in my application.
  2. I can get away with a non-generic interface in this instance, though it preferrable with it, but I'd like to explore the problem for the sake of learning.

Feel free to adjust the title if there's a better way to describe this problem, I'm unsure.

Brodie007
  • 3
  • 1
  • How far can you get with using `Item>`? – Ole V.V. Oct 14 '16 at 09:55
  • That'll work. My main interest was in querying about any alternative designs that may not require doing > in every place it's used. If that's the recommended approach, I'm happy to oblige however. – Brodie007 Oct 14 '16 at 10:15

1 Answers1

0

There is a different response to each warning, so a different remedy is required as well.

1: The Interface:

public static interface Item<T extends Item> { public T Copy(); }

This is the key functionality of your Item class. You want each parameterized instance of Item to copy itself into an instance with the same parameterization. This first warning shows you you failed to enforce this.

You should have asked explicitely that T be the same type like so:

public static interface Item<T extends Item<T>> {
     public T Copy();
}

2: The default Item:

public static final Item DEFAULT_ITEM = new FooItem(Color.RED);

The problem with your default Item in the interface, is that because it is raw, it does not follow that contraint. Its copy() method may return an Item with a different parameter. If you want to enforce this on that Item, you need to declare it as a FooItem like so:

public static final FooItem DEFAULT_ITEM = new FooItem(Color.RED);

If you did not want to expose the full signature which may provide public or ptotected methods that you fell a default Item user shoulnd't see, you could hide that under an intermediate abstract type.


3: The RepairMan

You need to ask yourself some design questions:

Are all RepairMen able to repair any Item? In this case, You need to express that by using a wildard <?> on all your repair man fields and methods.

public static class RepairMan {
    Item<?> itemNeedingRepairs; // Any Item is acceptable
    public RepairMan(final Item<?> item) { // Any Item is acceptable
        itemNeedingRepairs = item.Copy();
    }       
}

Are some RepairMen specialized, like a Plumber and an Alectrician, wich should restrict the types of Item they can repair? In this case you must let the RepairMen set some bounds on the Item parameter.

public static class RepairMan<T extends Item<T> {
    TitemNeedingRepairs;
    public RepairMan(final Titem) {
        itemNeedingRepairs = item.Copy();
    }       
}

Example implementation using bounds:

public static interface ElectricItem extends Item<ElectricItem>{}

public static class LightBulb implements ElectricItem{
    @Override
    public LightBulb Copy() {
        return new LightBulb();
    }
}

public static class Electrician extends RepairMan<ElectricItem>{
    ElectricItem itemNeedingRepairs; // Only electric Items acceptable
    public Electrician(ElectricItem item) {
        super(item);
    }
}

public static void main(String[] argc){
    LightBulb bulb = new LightBulb();
    new Electrician(bulb);
}

Final words

Doing work in constructors is considered bad practive, and a RepairMan doesn't need yo know in advance which Item instance it'll work on. Instead, it should offer a repair method:

public void repair(Item<?> item){...}
Community
  • 1
  • 1
MrBrushy
  • 680
  • 3
  • 17
  • All excellent points. I arrived at solution #2 a while back, I hadn't known Java would infer the concrete type from T in copy(). This allowed me to remove all templating for my needs. The repairman was just a result of struggling to deliver the question in a concise way, which by itself could have indicated a flaw. Nevertheless, thank you very much. – Brodie007 Oct 28 '16 at 00:50