Constructor with objects

Question

My code deals with two classes: Point and Line. Line consists two Points:

import java.util.Scanner;

public class Line {
Scanner scan = new Scanner (System.in);
private Point p1,p2;

public Line(int x1, int x2, int y1, int y2){
    p1=new Point(x1,y1);
    p2=new Point (x2,y2);

Now I have to make the constructor, but I'm afraid this code is not legal due to Aliasing:

public Line (Point p1, Point p2){
    this.p1=p1;
    this.p2=p2; 
    Line a1=new Line (p1.getX(),p1.getY(),(p2.getX()),p2.getY());

string toString method:

public String toString()
{
    return "Line between: (" +  p1 + p2 + ")"; 
}

The main is:

Point a=new Point (1,2);
    Point b=new Point (2,3);
    Line x=new Line(a,b);
    System.out.println(a);
    System.out.println(b);
    System.out.println(x);

The output here comes out as it should, but my question is: 1) The constructor is ok? Does is cause aliasing? 2) the String toString method is legal?

Thank you for answering.

Line a1 in the second constructor does not make sense. This creates a second line (and discards it once the constructor is finished). — tobias_k
– tobias_k, Commented Jan 4, 2013 at 17:14
Creating a new instance of Scanner with the same InputStream (System.in in your case) for each instance of Line is a bad practice, IMO at least. — amit
– amit, Commented Jan 4, 2013 at 17:15
@tobias_k - I created the second Line so I won't cause alasing and expose the private attributes of Point class. Was it wrong? — Assaf
– Assaf, Commented Jan 4, 2013 at 17:18
@user1941081 unless there's something between Line a1...; and the end of the constructor, than that line does absolutely nothing. — Sam I am says Reinstate Monica
– Sam I am says Reinstate Monica, Commented Jan 4, 2013 at 17:22

Sam I am says Reinstate Monica · Accepted Answer · 2013-01-04 17:16:09Z

Whether or not the constructor is okay depends on your requirements except for 1 part

Line a1=new Line (p1.getX(),p1.getY(),(p2.getX()),p2.getY());

if that line of code has a purpose, than that purpose is probably left out of your samples, because a1 will go out of scope when the constructor finishes unless you do something with it.

What your constructor does, is take the points that you declare in your main, and it puts them in Line x. If you do something like

Line x=new Line(a,b);
Line y=new Line(a,b);

than Line y has the same points that Line x has, and when you change the values of those points of one, you change it for the other.

now, if you're worried about that, than you can do something like

public Line (Point p1, Point p2){
    this.p1=new Point(p1.getX(), p1.getY);
    this.p2=new Point(p2.getX(), p2.getY); 
    ...

and that will just make a copy of the line

Nathan Hughes · Accepted Answer · 2013-01-04 17:35:10Z

You should change the constructor to:

public Line (Point p1, Point p2){
    this(p1.getX(),p1.getY(),(p2.getX()),p2.getY());
}

The local a1 variable doesn't make sense (as was previously pointed out).

Really you could turn around how the constructors are invoked and have the main constructor be the one with the Points. That would simplify it to:

public Line(Point p1, Point p2) {
    this.p1 = p1; this.p2 = p2;
}

public Line(int x1, int y1, int x2, int y2) {
    this(new Point(x1, y1), new Point(x2, y2);
}

Sam I am makes a good point about what happens if a Point changes (+1 from me). One easy way to deal with this is to make the Points and Lines immutable.

Also the toString method is fine. p1 and p2 are instance variables, if there is no local variable or parameter with those names then the compiler looks for an instance variable with a matching name.

Thank you, changed it and it works, it does make more sense. I think I have to get over the alasing idea again and see if I understand where to create a new object and where not to.

Collectives™ on Stack Overflow

Constructor with objects

2 Answers 2

Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Related