Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clone() method does not declare throws CloneNotSupportedException #98

Closed
damianszczepanik opened this issue May 23, 2015 · 7 comments
Closed
Assignees

Comments

@damianszczepanik
Copy link

I have two classes Animal and Dog. Animal is declared as abstract and Dog extends Animal.

i was trying to add builders for both but I found that Animal.clone() method does not declare CloneNotSupportedException in method definition so sub class Dog cannot be compiled with success:

  public Object clone() {
    try {
      Dog result = (Dog)super.clone();
      result.self = result;
      return result;
    } catch (CloneNotSupportedException e) {
      throw new InternalError(e.getMessage());
    }
  }

and the problem is that parent class Animal declares

  public Object clone() {

while it should (as it is for java.lang.Object)

protected native Object clone() throws CloneNotSupportedException;

Solutions:

  • declare missing throws block
  • add option to configuration file so I can choose whether clone() method should be implemented or not
  • not catch such exception if base class is declared as pojobuilder (you know it from withBaseclass)
@mkarneim
Copy link
Owner

Damian,

I am not sure if I understand your issue correctly.

May I ask you for clarification?

  1. The code example above looks like it's declared in class Dog (since the clone method's return value is being casted to Dog). Is that right? Or did you mean DogBuilder?

  2. You say you want to generate a builder for an abstract class (Animal). This is only supported if you place the annotation onto a (concrete) factory method.
    Anyway, it would be better if PB did mark 'badly' placed annotations on abstract classes as errors in the first place.

I guess you want to create a builder class hierarchy. Is that right? If so, why?

@damianszczepanik
Copy link
Author

Hi Michael,

  1. It's all about builders and this code comes from DogBuilder that calls super.clone() (clone from Animal)
  2. Yes, I want to create builders for class hierarchy to be able to use withXXX method implemented in AnimalBuilder when I create DogBuilder

Eg.

abstract class AnimalBuilder {
    withAge(int age) {...}
}
class DogBuilder extends AnimalBuilder {
    withSpeed(int speed) {...}
}

so in test I can use method from abstract class when using concrete builder:

new DogBuilder().withSpeed(34).withAge(2).build();

In that case I can reuse withAge method in Dog, Cat... Dragon, Monster etc :) Of course creating abstract Animal has no sense but reusing method from this class does.

@mkarneim
Copy link
Owner

Damian,

I have to apologize that I haven't answered to your last comment for so long. I had a lot of work to do.

So, going back to your issue, I have created a sample project and just entered the following classes:

public abstract class Animal {
  private int age;

  public int getAge() {
    return age;
  }

  public void setAge(int age) {
    this.age = age;
  }
}

@GeneratePojoBuilder
public class Dog extends Animal {
  private int speed;

  public int getSpeed() {
    return speed;
  }

  public void setSpeed(int speed) {
    this.speed = speed;
  }
}

I hope that this is what you are talking about, isn't it? Animal is abstract and Dog is a subclass of Animal.

Please note, that the @GeneratePojoBuilder annotation is only placed on Dog but not on Animal. And you don't need to use any withBaseclass attribute.
A side note: withBaseclass is not intended to be used to create a builder hierarchy, but instead to inherit common utility methods into all generated builders. Actually you never have to create a builder hierarchy since a single builder always contains every property of the pojo, independent of the location of its declaration.

As you see, the generated DogBuilder can be used exactly as you have required:

new DogBuilder().withSpeed(34).withAge(2).build();

Since there is only one builder (DogBuilder), you don't have any problems with the clone() method.

Does this solve your issue?

@damianszczepanik
Copy link
Author

Hi Michael,

In the meantime I have tried to prepare PR to extend pojobuilder and cover this issue. Lucky me I was working with real project so I could test all cases. Finally I gave up because of the following issue:

  • getAge method is public while setAge does not exist because age value should not be exposed - this is set by hibernate and reflection and nobody should set this value except builder
  • Animal and Dog do not belong to the same package

Having two above (or maybe only first is the key) I found that external tool which you use to list available fields does not list age and thus does not provide withAge` method. And that's made my unhappy because I believe pojobuilder cannot satisfy my about this.

Perfect solution for me would be to be able to list all fields (no matter what modifier it has and what methods related to this fields are implemented). If the attribute has setter then setter method can be used to change value, otherwise reflection.

@kussm
Copy link

kussm commented May 14, 2016

Hi Michael,

I got the same problem, when I was trying to use a factory returning the appropriate builder for subclasses of a common type. The correct approach is of course to let the builder implement a common interface. But this interface must be manually adjusted after each change on the base class, since PojoBuilder does not (yet?) support generating an interface for its generated builder.

As a workaround I tried to let the builders just extend the base class builder -- of course I know that this generates duplicate fields in each builder type of the inheritance chain, but in my situation this is only for testing purpose, so I don't care.

`
@GeneratePojoBuilder
public class Vehicle {
double maximumSpeed;
double weight;
}

@GeneratePojoBuilder(withBaseclass=VehicleBuilder.class)
public class Car extends Vehicle {
}

@GeneratePojoBuilder(withBaseclass=VehicleBuilder.class)
public class Truck extends Vehicle {
}

public class BuilderFactory {
VehicleBuilder create() {
return new TruckBuilder(); /* or new CarBuilder() */
}
}

public class Factory {
// returns a type determined by BuilderFactory
public Vehicle create() {
return new BuilderFactory().create().withMaximumSpeed(100).withWeight(12).build();
}
}
`

@drekbour
Copy link
Contributor

I have not read all of the above but #132 includes Clone method must only catch CloneNotSupportedException if thrown by superclass and probably closes this? It at least supports Animal-Dog builder inheritance.

mkarneim added a commit that referenced this issue Apr 30, 2017
@mkarneim mkarneim self-assigned this Apr 30, 2017
@mkarneim
Copy link
Owner

This should now be fixed in PB 3.6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants