I'm having trouble dealing with a design in Java for the following description: I need to create a guitar shop that provides tuning service for different guitars. Tuning is done differently for each guitar type. I need to provide a design (classes and relations) that can deal with a basic scenario where a guitar needs to be tuned. So I got: Guitar abstract\interface 2 Guitar implementations (Gibson and Fender) I got my GuitarShop GuitarService (interface\abstract) GibsonService (implements GuitarService) FenderService (implements GuitarService) GuitarService factory So basically the factory gets the guitar type and returns the required guitar service. But the guitar service contains a method: public void tuneGuitar(Guitar guitarToTune) {...} Which means I need to cast my Guitar every time in each service (each guitar is tuned differently). I tried using generics by templating GuitarService with But then my factory cannot serve me anymore (I cannot safely instantiate objects with parameters determined at run time). Is there a best practice for such a scenario?

Design suggestion, failed to combine factory with generics

Answers (4)

Reputation: 5055

Below I provided the solution you've asked for. But I'm not sure, if that is the solution you really need for your problem. Because I can't understand, why you need to model the properties of the different guitars by class type. But I can only assume.

Approach 1 | Cast + recompilation per new service

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

public class Main {

    public static void main(String[] args) {
        Guitar f = new Fender();
        Guitar g = new Gibson();

        TuneService<Guitar> fts = TuneServices.getService(f);
        fts.tune(f);

        TuneService<Guitar> gts = TuneServices.getService(g);
        gts.tune(g);
    }

    static interface Guitar {
    }

    static class Fender implements Guitar {
    }

    static class Gibson implements Guitar {
    }

    // tune service that accepts only guitars as generic types
    static interface TuneService<T extends Guitar> {
        public void tune(T guitar);
    }

    static class FenderService implements TuneService<Fender> {
        @Override
        public void tune(Fender guitar) {
            System.out.println("Tune Fender.");
        }
    }

    static class GibsonService implements TuneService<Gibson> {
        @Override
        public void tune(Gibson guitar) {
            System.out.println("Tune Gibson.");
        }
    }

    // factory class
    static final class TuneServices {
        private static final Map<Class<?>, TuneService<?>> services;
        static {
            Map<Class<?>, TuneService<?>> tmp = new HashMap<Class<?>, TuneService<?>>();
            tmp.put(Fender.class, new FenderService());
            tmp.put(Gibson.class, new GibsonService());

            // populate services
            services = Collections.unmodifiableMap(tmp);
        }

        public static <T extends Guitar> TuneService<T> getService(T guitar) {
            TuneService<?> s = services.get(guitar.getClass());
            if (s == null)
                throw new IllegalArgumentException();

            // cast is safe at this point
            @SuppressWarnings("unchecked")
            TuneService<T> ts = (TuneService<T>) s;
            return ts;
        }
    }
}

Approach 2 | ServiceLoader + cast + no recompilation per new service

With this approach you don't have to recompile the factory as you add new guitar services, you can even extend this approach in order to avoid recompiling, when you add a new guitar.

ServiceLoaderExample.java

package guitar;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.ServiceLoader;

public class ServiceLoaderExample {

    public static void main(String[] args) {
        Guitar f = new Fender();
        Guitar g = new Gibson();

        TuneService<Guitar> fts = TuneServices.getService(f);
        fts.tune(f);

        TuneService<Guitar> gts = TuneServices.getService(g);
        gts.tune(g);
    }

    static interface Guitar {
    }

    static class Fender implements Guitar {
    }

    static class Gibson implements Guitar {
    }

    // tune service that accepts only guitars as generic types
    static interface TuneService<T extends Guitar> {
        public void tune(T guitar);
    }

    // factory class
    static final class TuneServices {
        private static final Map<Class<?>, TuneServiceProvider<?>> providers;
        static {
            @SuppressWarnings("rawtypes")
            ServiceLoader<TuneServiceProvider> loader = ServiceLoader.load(TuneServiceProvider.class);

            Map<Class<?>, TuneServiceProvider<?>> tmp = new HashMap<Class<?>, TuneServiceProvider<?>>();
            for (TuneServiceProvider<?> s : loader) {
                tmp.put(s.getGuitarType(), s);
            }

            // populate providers
            providers = Collections.unmodifiableMap(tmp);
        }

        public static <T extends Guitar> TuneService<T> getService(T guitar) {
            TuneServiceProvider<?> p = providers.get(guitar.getClass());
            if (p == null)
                throw new IllegalArgumentException();

            TuneService<?> s = p.getService();

            // cast is safe at this point
            @SuppressWarnings("unchecked")
            TuneService<T> ts = (TuneService<T>) s;
            return ts;
        }
    }
}

TuneServiceProvider.java

package guitar;

import guitar.ServiceLoaderExample.Guitar;
import guitar.ServiceLoaderExample.TuneService;

public interface TuneServiceProvider<T extends Guitar> {
    public TuneService<T> getService();

    public Class<T> getGuitarType();
}

FenderTuneServiceProvider.java

package guitar;

import guitar.ServiceLoaderExample.Fender;
import guitar.ServiceLoaderExample.TuneService;
import guitar.TuneServiceProvider;

public class FenderTuneServiceProvider implements TuneServiceProvider<Fender> {

    @Override
    public TuneService<Fender> getService() {
        return new FenderService();
    }

    @Override
    public Class<Fender> getGuitarType() {
        return Fender.class;
    }

    static class FenderService implements TuneService<Fender> {
        @Override
        public void tune(Fender guitar) {
            System.out.println("Tune Fender.");
        }
    }
}

GibsonTuneServiceProvider.java

package guitar;

import guitar.ServiceLoaderExample.Gibson;
import guitar.ServiceLoaderExample.TuneService;
import guitar.TuneServiceProvider;

public class GibsonTuneServiceProvider implements TuneServiceProvider<Gibson> {

    @Override
    public TuneService<Gibson> getService() {
        return new GibsonService();
    }

    @Override
    public Class<Gibson> getGuitarType() {
        return Gibson.class;
    }

    static class GibsonService implements TuneService<Gibson> {
        @Override
        public void tune(Gibson guitar) {
            System.out.println("Tune Gibson.");
        }
    }
}

/META-INF/services/guitar.TuneServiceProvider must be on classpath or main/java/resources if you use maven

guitar.FenderTuneServiceProvider
guitar.GibsonTuneServiceProvider

Output

The output is identical for both approaches.

Tune Fender. 
Tune Gibson.

Drawback

Both approaches have the same drawback, if the service can be accessed by the client.

TuneService<Guitar> fts = TuneServices.getService(f);
fts.tune(f);

TuneService<Guitar> gts = TuneServices.getService(g);
gts.tune(g);

// use gibson service with fender
gts.tune(f);

A ClassCastException can be thrown at runtime, if the services are used with the super type Guitar and e.g. a FenderService is used with a Gibson and vice versa. To avoid this, the service invocation could be encapsulated in a method, the get the right service (this method is already there) and applies it on the guitar.

Upvotes: 1

iozee

Reputation: 1349

I tried using generics by templating GuitarService with But then my factory cannot serve me anymore (I cannot safely instantiate objects with parameters determined at run time).

You can implement it like this:

// declare a common interface
public interface GuitarTuningService<GuitarGenericType extends Guitar> {
  void tune(GuitarGenericType guitar)
}

public class FenderTuningService implements GuitarTuningService<FenderGuitar> {
  public void tune(FenderGuitar guitar) {
    // tune it
  }
}

The factory in turn could be abstract:

public abstract GuitarServiceFactory<T extends GuitarTuningService> {
  T getGuitarService();
}

// and the concrete implementation:
public class FenderServiceFactory extends GuitarServiceFactory<FenderTuningService> {
  public FenderTuningService getGuitarService() {
    // instantiate & return FenderTuningService 
  }
}

And finally we can use this stuff like this:

public class GuitarTuner {

  private GuitarServiceFactory factory;
  private Guitar guitar;

  public GuitarTuner(Guitar guitar) {
    this.guitar = guitar;
    switch (guitar.getClass()) {
      case Fender.class:
        factory = new FenderServiceFactory();
        break;
      // other guitar types
      throw new IllegalArgumentException("Unknown guitar type added");
    }
  }

  public void tune() {
    factory.tune(guitar);
  }
}

Upvotes: 0

Zymus

Reputation: 1698

You could do

public interface GuitarTuningService<T extends Guitar> {

    void tune(T guitar);
}

And then

public class FenderTuningService implements GuitarTuningService<Fender> {

    public void tune(Fender guitar) {
        // do stuff
    }
}

And in the factory

public class GuitarTuningServiceFactory {

    public GuitarTuningService getTuningService(final Guitar guitar) {
        if (guitar instanceof Fender) {
            return fenderTuningService;
        }
        // if guitar is a Gibson, return GibsonTuningService
    }

    private final GuitarTuningService<Fender> fenderTuningService
        = new FenderTuningService();
}

Upvotes: 0

John Bollinger

Reputation: 180331

The main issue is that your Guitar interface hides exactly the details that your factory needs to use to choose the appropriate GuitarTuningService implementation. One way to approach the problem would be to implement the Visitor pattern, a.k.a. double dispatch. The Guitar interface would get the following method:

public void accept(GuitarVisitor visitor);

Each Guitar implementation would, separately, implement the method the same way:

public void accept(GuitarVisitor visitor) {
    visitor.visit(this);
}

The GuitarVisitor interface would look like this:

public interface GuitarVisitor {
    public void visit(Fender guitar);
    public void visit(Gibson guitar);
    // other Guitar types as necessary
}

An implementation of GuitarVisitor performs the appropriate guitar-type specific behavior in each method. For example, the GuitarTuningServiceFactory could use an inner GuitarVisitor implementation to select the appropriate GuitarTuningService for each provided guitar. You cannot avoid a cast (though you can avoid type-safety warnings by performing it via Class.cast()), but you can safely perform it just once at the beginning of each tune() method, recording the downcast reference.

The main drawback of the Visitor pattern is that it locks in the possible types of the specific visited classes. You can add a catch-all method to partially mitigate that (public void visit(Guitar guitar)).

Upvotes: 2