Prototype pattern re-factor

[panksy2k]

Hi,

The Prototype implementations where clone() is Overridden (ElfBeast, ElfMage etc) can rather not have implementation of clone(). Instead their respective parent class (Mage, Beast) can have a general implementation of clone which creates a new instance by doing a shallow copy (Object.clone()) and then subsequently type-cast to the type of class (Mage.class or Beast.class etc) it is cloning for.

This will have advantage of:

1. Removing verbosity among all implementations and dedicate the responsibility of cloning the object by their respective interfaces (abstract class).

2. Avoid using 'new' more than once and using only whilst cloning the prototype object that is passed in HeroFactoryImpl.

Regards, panksy2k

[ghost]

Hi @panksy2k , Say we move the clone implementation from ElfBeast to Beast by doing this.

public Beast clone() throws CloneNotSupportedException { return (Beast)( super.clone() ); }

Then if we call clone method on an object of ElfBeast, it will return an Object of type Beast, not ElfBeast. That is why, clone() implementation should be in ElfBeast class.

[jwheeler31]

@panksy2k @ghost, Adding clone() method (copy() in this case), fights inheritance/polymorphism. Any sort of instantiation of an abstract class should throw a compile-time error. The current implementation should stand as is (only a concrete class is instantiated). If Beast were to have any attributes to be initialized, it should be done via constructor inheritance.

[iluwatar]

Under construction by https://github.com/jasciiz

[ohbus]

@jasciiz can you please share any progress you have made for this issue?

[barathgdkrish]

@ohbus : Can I pick up this issue? It is not assigned to anyone now.

[ohbus]

Sure thing @barathgdkrish.

Please mention a timeline for when can we expect a PR against this issue.

[barathgdkrish]

Hi @ohbus , I have forked and cloned the repo. This is my first contribution, I'll study the ask and the code and will let you know the timeline by tomorrow EOD?

[vrushofficial]

Hey @ohbus, @barathgdkrish is still working on this or shall I start with this?

[barathgdkrish]

Hi @vrushabhjoshi, I'm still working on this.

---

From: Vrushabh Joshi @.***> Sent: Wednesday, 28 July 2021, 12:26 To: iluwatar/java-design-patterns Cc: Barath G D Krishnan; Mention Subject: Re: [iluwatar/java-design-patterns] Prototype pattern re-factor (#584)

Hey @ohbushttps://github.com/ohbus, @barathgdkrishhttps://github.com/barathgdkrish is still working on this or shall I start with this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/iluwatar/java-design-patterns/issues/584#issuecomment-888061654, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOOLDWMGXNS6NJDK334RS3LTZ6S23ANCNFSM4DNGJTXA.

[ghost]

How about adding a default method in typed prototype abstract class itself?

@Slf4j
public abstract class Prototype<T> implements Cloneable {

  public T copy() {
    try {
      return (T) super.clone();
    } catch (CloneNotSupportedException e) {
      LOGGER.error("Failed to clone {}!", this.getClass().getName());
    }
    return null;
  }
}

Doing changes with respect to this class works fine and all test were passed. Not sure whether this is a good practice or not.

[ThexXTURBOXx]

Hi, if noone else is working on this, I could pick up this issue. Is that okay? :)

[iluwatar]

Yes, since there is no sign of PR from @barathgdkrish you can go ahead @ThexXTURBOXx

[yonatankarp]

There's a pr for the fix, but it seems that SonarCloud isn't happy with the try-catch of the clone in terms of coverage. From checking online, it's considered anti-pattern to test this... Any suggestions?