Jump to content

  • Log In with Google      Sign In   
  • Create Account


Multiple constructors for Animation class has duplicated code. How to fix code duplication?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
9 replies to this topic

#1 warnexus   Prime Members   -  Reputation: 1394

Like
0Likes
Like

Posted 06 January 2014 - 04:15 PM

Edit: The code is in Java. My apologies for forgetting to say this...

 

This is the entire animation system for my 2D game. There are 2 constructors with different parameters for the Animation functionality that do the same work in their respective constructor. The first one only has the string array which would contain directory paths for the image file. The second one would also have the string array parameter but also a sprite reference because it would later on be drawn relative to that sprite reference after loading the art assets.

 

The first constructor is used for things that do not need to be placed relative to a sprite. A button prompt from a dialogue box used this constructor. Objects that use this constructor will need to override their draw method to draw this object because this type of animation does not need to be drawn relative to a sprite object.

 

The second constructor is used for the things like character animations: idle animation, attack animation, running animation all used the second constructor for animation because these type of animations need to be drawn relative to a sprite object. Objects that use the second constructor will use the draw method from the Animation class.

 

Is there a way around the duplication code?

public class Animation {
 
private BufferedImage image;
private ArrayList<Image> animList;
private int currentFrame;
private int[] frameEntries;
private int entries = 10;
private int frameInterval = 6;
private Sprite sprite;
 
public Animation(String[] animations)
{
animList = new ArrayList<>();
// load the images
 
for(int i = 0; i < animations.length;i++)
{
try {
image = ImageIO.read(new FileInputStream(new File(animations[i])));
 
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
animList.add(image);
 
}
 
frameEntries = new int[animList.size()];
 
 
for(int i = 0; i < animList.size();i++)
{
frameEntries[i] = entries;
entries += frameInterval;
}
}
 
 
public Animation(Sprite sprite, String[] animations)
{
this.sprite = sprite;
animList = new ArrayList<>();
// load the images
 
for(int i = 0; i < animations.length;i++)
{
try
{
 
image = ImageIO.read(new FileInputStream(new File(animations[i])));
}
catch(Exception e)
{
e.printStackTrace();
 
}
animList.add(image);
}
 
 
 
 
frameEntries = new int[animList.size()];
 
 
for(int i = 0; i < animList.size();i++)
{
frameEntries[i] = entries;
entries += frameInterval;
}
}

public void draw(Graphics g)
{
 
for(int i = 0; i < animList.size();i++)
{
 
if(currentFrame <=  frameEntries[i])
{
 
g.drawImage(animList.get(i),(int)sprite.getX() ,(int)sprite.getY()  ,null);
break;
}
}
}

}


Edited by warnexus, 06 January 2014 - 05:32 PM.


Sponsor:

#2 Álvaro   Crossbones+   -  Reputation: 12362

Like
0Likes
Like

Posted 06 January 2014 - 04:31 PM

Since you are not using initializer lists, it means all your data members are first constructed using default constructors and then you do something to them. In that case, you can easily pull the common parts of the construction to a private function called something like `initialize' and make the constructors call it.

If you wanted to construct the members with non-default constructors (using an initializer list), the problem would be trickier. You would have to use a feature of C++11 called delegating constructors.

#3 ferrous   Members   -  Reputation: 1797

Like
0Likes
Like

Posted 06 January 2014 - 04:40 PM

I don't think he's using C++?

 

This is a guess, but is it C#?  I think it is?  (I'm guessing because of the private int[] frameEntries;) line

 

If so you could do something like 

public Sample(string str) : this(int.Parse(str)) {
}

Edited by ferrous, 06 January 2014 - 04:45 PM.


#4 warnexus   Prime Members   -  Reputation: 1394

Like
0Likes
Like

Posted 06 January 2014 - 05:32 PM

Code is in Java. I have made a note for other people who have not read the post but have experience with the Java language.



#5 AlanSmithee   Members   -  Reputation: 971

Like
1Likes
Like

Posted 06 January 2014 - 06:05 PM

Java and C# are verly much alike in syntax..

 

Just a minor change from what ferrous said

public class A
{
  private int a;
  private String s;

  public A()
  {
    this(1);
  }

  public A(int a)
  {
    this(a, "something");
  }

  public A(int a, String s)
  {
    this.a = a;
    this.s = s;
  }
}

(A not so useful example, but you get it the just of it, I hope)

 

The constructors are differentiated by their signature (arguments) so you should be able to cascade the constructor calls in a nice way to suit your needs. The general rule of thumb is to chain from fewest arguments to most arguments.

 

In java,there is also a thing called "variable length argument lists" http://www.deitel.com/articles/java_tutorials/20060106/VariableLengthArgumentLists.html, it might be something that you can have use of in some situation...


Edited by AlanSmithee, 06 January 2014 - 06:19 PM.


#6 warnexus   Prime Members   -  Reputation: 1394

Like
0Likes
Like

Posted 06 January 2014 - 07:01 PM

Java and C# are verly much alike in syntax..

 

Just a minor change from what ferrous said

public class A
{
  private int a;
  private String s;

  public A()
  {
    this(1);
  }

  public A(int a)
  {
    this(a, "something");
  }

  public A(int a, String s)
  {
    this.a = a;
    this.s = s;
  }
}

(A not so useful example, but you get it the just of it, I hope)

 

The constructors are differentiated by their signature (arguments) so you should be able to cascade the constructor calls in a nice way to suit your needs. The general rule of thumb is to chain from fewest arguments to most arguments.

 

In java,there is also a thing called "variable length argument lists" http://www.deitel.com/articles/java_tutorials/20060106/VariableLengthArgumentLists.html, it might be something that you can have use of in some situation...

I won't be able to use the code example above because the first constructor is not suppose deal with sprites. Its job is to only deal with an array of strings



#7 ferrous   Members   -  Reputation: 1797

Like
0Likes
Like

Posted 06 January 2014 - 07:45 PM

 

Java and C# are verly much alike in syntax..

 

Just a minor change from what ferrous said

public class A
{
  private int a;
  private String s;

  public A()
  {
    this(1);
  }

  public A(int a)
  {
    this(a, "something");
  }

  public A(int a, String s)
  {
    this.a = a;
    this.s = s;
  }
}

(A not so useful example, but you get it the just of it, I hope)

 

The constructors are differentiated by their signature (arguments) so you should be able to cascade the constructor calls in a nice way to suit your needs. The general rule of thumb is to chain from fewest arguments to most arguments.

 

In java,there is also a thing called "variable length argument lists" http://www.deitel.com/articles/java_tutorials/20060106/VariableLengthArgumentLists.html, it might be something that you can have use of in some situation...

I won't be able to use the code example above because the first constructor is not suppose deal with sprites. Its job is to only deal with an array of strings

 

 

I think you can, looking at your code, there seems to be only one real difference between the two constructors.  (Also I indented your code, because I found it nearly unreadable)

public Animation(Sprite sprite, String[] animations)
{
    this.sprite = sprite;
    this(animations)
}

public Animation(String[] animations)
{
    animList = new ArrayList<>();
    // load the images
 
    for(int i = 0; i < animations.length;i++)
    {
        try 
        {
            image = ImageIO.read(new FileInputStream(new File(animations[i])));
        } 
        catch (IOException e) 
        {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        animList.add(image);
 
    }
 
    frameEntries = new int[animList.size()];
 
 
    for(int i = 0; i < animList.size();i++)
    {
        frameEntries[i] = entries;
        entries += frameInterval;
    }
}
 
 

Edited by ferrous, 06 January 2014 - 07:46 PM.


#8 warnexus   Prime Members   -  Reputation: 1394

Like
0Likes
Like

Posted 06 January 2014 - 08:56 PM

Oh yeah. That would work. Thanks.

 

Turns out you have to swap these two around so the correct code should be:

 

this(animations);

this.sprite = sprite;


Edited by warnexus, 06 January 2014 - 09:01 PM.


#9 AlanSmithee   Members   -  Reputation: 971

Like
1Likes
Like

Posted 07 January 2014 - 04:50 AM

Yes, a call to another constructor must always be on the first line, forgot to mention that, sorry.



#10 ferrous   Members   -  Reputation: 1797

Like
0Likes
Like

Posted 07 January 2014 - 01:44 PM

Yes, a call to another constructor must always be on the first line, forgot to mention that, sorry.

 

Well, I learned some Java from this thread =)






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS