views:

177

answers:

7

I had a quick look at the "Related Questions" suggested but I couldn't find one directly related to what I'm asking. Even if there was one, I'd still appreciate your opinion on the best way to do this.

First some context.
I'm working on extending a Java Application and I have the following Interface and Class:

public interface Mapper {
  public void foo();
}

public class SomeGrammar implements Mapper {

  public SomeGrammar() {}

  public SomeGrammar(SomeGrammar toCopy) {
    //Copy specific fields
  }

  public void foo() {}
}

The application I'm using was built with only one grammar type in mind, SomeGrammar, and as such the SomeGrammar class is woven throughout through the application.

SomeGrammar objects are often copied using a copy constructor:

SomeGrammar aGrammar = new SomeGrammar();
SomeGrammar newGrammar = new SomeGrammar(aGrammar);

I'm attempting to implement new types of grammars. So I was going to pull the core methods out and implement a Grammar interface, or even consolidate those methods into the Mapper interface. So all grammar classes would implement this interface.


Anyway, my question to you is, what is the best way to go about replacing the above line so that I can generalise the copying of whatever grammar object happens to be currently being referenced by the Interface's Type?

Obviously I can't

Grammar newGrammar = new Grammar(aGrammar);

since Grammar is the Interface's Type, and new Grammar(aGrammar) makes no sense. I suppose I could add a cloning method to my interface to use instead of a copy constructor, but as I said before, I'd appreciate your opinion on the matter.

Thanks in advance,
Eoin

+3  A: 

If I understood your question correctly, my solution would look something like the following :

Lets say we had 3 concrete grammars we wished to create, GrammarA, GrammarB, and GrammarC. We would define each of those classes to implement the Grammar interface. The GrammarInterface would have your foo() function, and also a doCopy() function defined. Each of those concrete Grammar classes would then be responsible for implementing the foo() capability and for returning a copy of themselves by calling a copy constructor. Is this what you were looking for?

interface IGrammar { 

    public void foo();
    public Grammar doCopy();

}

class GrammarA implements IGrammar { 

    public Grammar doCopy(Grammar g) { 

        return new GrammarA(g);

    }

}
Amir Afghani
Java's covariance allows doCopy of GrammarA to return an instance of GrammarA.
Fedearne
Yes, you understood correctly. By a 'cloning method' I meant a method which creates a copy of the object (just as you've shown me). Sorry for my bad terminology, good answer :-)
Eoin Murphy
Eoin, you may want to choose the answer as the 'accepted' solution if this resolves your issue.
Amir Afghani
A: 

You can add a clone() as part of your interface, and have the concrete classes implement it properly. Then your client code will look like:

Grammar aGrammar = new SomeGrammar();
Grammar newGrammar = aGrammar.clone();

Check out a bit of explanation here. Personally, I would just recommend having your own copy() method as part of Grammar interface. It's better than reusing clone() IMHO.

notnoop
That won't help in the case of instantiating a different type of grammar: AnotherGrammer newGrammear = new AnotherGrammer(aGrammar);Whether or not that's really what the OP should be doing is another matter.
Pavel
I think the above answer would work. All I'm trying to do is to create a copy of an object (whose class implements the interface) when the reference type of the object is that of the interface. So as long as each of the different types of grammar classes had clone() implemented correctly, then surely this would be fine.And yes, others have expressed that using a copy() method would be better than clone().
Eoin Murphy
A: 

I think what you want is to have the Grammar interface include a makeCopy signature.

Then making a copy would look like:

Grammar newGrammar = oldGrammar.makeCopy();

Avoid using the clone method, for that way lies madness!

Naganalf
A: 

IMO the best option is to get to have public interface Mapper extends Cloneable { public Object clone(); }

And have that method call super.clone() and perform deep cloning if relevant (only the implementation class knows about that, in principle).

Resorting to reflectivity will probably hit performance and make extension design a bit more (too?) complex. Another option here would be to provide with an abstract class that features the copy constructor instead of an interface, but that has obvious limitations that could be a show stopper.

Shinigami
Use of **clone()** is no longer recommended (e.g. See Joshua Bloch's **Effective Java**)
alasdairg
A: 

You could also use reflection to find the type of Grammar and create a new instance of this type using the matching constructor:

interface Interface {
}

class A implements Interface {
    public A(Interface other) {

    }
}

class B implements Interface {
    public B(Interface other) {

    }
}

public class Test {
    public static Interface newWithExactType(Interface i) throws Exception {
     Class<? extends Interface> c = i.getClass();
     Constructor<? extends Interface> ctor = c.getConstructor(Interface.class);
     return ctor.newInstance(i);

    }
    public static void main(String[] args) throws Exception {
     Interface a = newWithExactType(new A(null));
     Interface b = newWithExactType(new B(null));
     System.out.println("Type of a: "+a.getClass().getSimpleName());
     System.out.println("Type of b: "+b.getClass().getSimpleName());
    }
}
Karl
+1  A: 

Object.clone() is bad, see Effective Java for discussion. Marker interfaces are bad (see Externalizable, Serializable, ...) there are tons of libraries out there which try to circumvent such useless marker interfaces. Cloneable<T> might be an approach.

I'd suggest a Factory:

 Grammar someGrammar = Factory.newGrammar();
 Grammar copyOf = Factory.copyGrammar(someGrammer);

The reasons for that is, the Factory can delegate to the grammar object if it's Cloneable, but it can do other things as well such as keep a list of creates grammars or returning adapters or returning cached instances (in general, stuff which is independent of the actual grammar implementation). Also, the implementation classes need not to be used directly throughout the whole application.

mhaller
It depends whether it's desirable to be able to separate being able to copy a grammar from the grammar implementation, so I've +1'd both your and Amir Afghani's answers.
Pete Kirkham
A: 

If you really need the ability to copy between multiple different implementations, I would suggest the following approach:

public interface Mapper {
    void Mapper();
    void Mapper(Mapper copy);

    Foo getFoo();
    Bar getBar();
    // getters for shared properties which can be copied between implementations

    void doStuff();
}

public abstract AbstractBaseMapper implements Mapper {
    protected Foo foo;
    protected Bar bar;

    public void Mapper() {}

    public void Mapper(Mapper copy) {
         foo = copy.getFoo();
         bar = copy.getBar();
    }

    public Foo getFoo() { return foo; }
    public Bar getBar() { return bar; }
}

public ConcreteMapper extends AbstractBaseMapper {
    public void doStuff() {
    }
}

public AnotherMapper extends AbstractBaseMapper {
    public void doStuff() {
    }
}

public YetAnotherMapper implements Mapper {
    // NB: this one is a straight implementation of the Mapper interface, doesn't inherit from AbstractBaseMapper...
}

Now you can do stuff like:

Mapper m1 = new YetAnotherMapper();
Mapper m2 = new ConcreteMapper(m1);
Mapper m3 = new AnotherMapper(m2);

The downside to this approach is that you'd have to define all the copyable fields in the base interface so that they are always guaranteed to be implemented. An alternative would be to use reflection as suggested by Karl. You can also look into Dozer which automates such reflective property copying.

Pavel