wood burning stoves 2.0*
The moose likes Beginning Java and the fly likes Can't find bug in my Rectangle class.  Think it might be the fault of my setup. Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Can Watch "Can New topic
Author

Can't find bug in my Rectangle class. Think it might be the fault of my setup.

Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

I was trying to make a Rectangle class that takes the lower left corner (an x and a y) and a width and a height. (All ints)

I found that all I would need was the x, y and the upper right corner, which I called upperX and upperY, to get the intersection of two rectangles and the union of two rectangles and see if they overlap.

I almost had it working, though, when I looked at the output, since I didn't know how to avoid it, I had printed out each intersection twice. I noticed that for 3 and 2 intersection, the height would be slightly different than for 2 and 3 intersection, though it shouldn't be.

I had noticed a problem in that my setUpperX() was like normal, with the this.upperX = upperX; and the same thing for setUpperY().

However, and I couldn't figure out how to alter this as you don't have a height given to you (you have to calculate it) when determining a union or intersection Rectangle so

getUpperX() was returning (x+width) and getUpperY() was returning (y+height).

I had realized that that could be a possible problem and had tried to fix it by using the variable upperX and upperY instead of the get methods so that I could avoid that mess, but still it didn't seem to work.

So, I commented out those lines that used the variable and added second getWidth() and getHeight() methods that should have fixed it, but now it's worse and it won't even recognize an intersection at all unless the two rectangles completely overlap.

I had kept the new methods but changed the code back so that it at least recognized the intersections again. However, I'm confused as to why my height is changing.






How many rectangles do you want?
4
What do you want the X coordinate to be?
0 0 1 1
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
0 0 1 1
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
1 1 1 1
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
1 1 2 2
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
Rectangle 1 x=0, y=0, width =1, height=1
Rectangle 2 x=0, y=0, width =1, height=1
Rectangle 3 x=1, y=1, width =1, height=1
Rectangle 4 x=1, y=1, width =2, height=2
Rectangle overlaps: 6
Rectangle 1 and Rectangle 2 intersect to form x=0, y=0, width =1, height=1
Rectangle 2 and Rectangle 1 intersect to form x=0, y=0, width =1, height=1
Rectangle 3 and Rectangle 4 intersect to form x=1, y=1, width =1, height=1
Rectangle 4 and Rectangle 3 intersect to form x=1, y=1, width =1, height=2
Bounding box: x=0, y=0, width =3, height=3


How many rectangles do you want?
5
What do you want the X coordinate to be?
0 0 1 1
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
1 1 5 5
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
3 3 2 2
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
1 9 1 1
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
What do you want the X coordinate to be?
1 0 3 3
What do you want the Y coordinate to be?
What width do you want?
What height do you want?
Rectangle 1 x=0, y=0, width =1, height=1
Rectangle 2 x=1, y=1, width =5, height=5
Rectangle 3 x=3, y=3, width =2, height=2
Rectangle 4 x=1, y=9, width =1, height=1
Rectangle 5 x=1, y=0, width =3, height=3
Rectangle overlaps: 5
Rectangle 2 and Rectangle 3 intersect to form x=3, y=3, width =2, height=3
Rectangle 2 and Rectangle 5 intersect to form x=1, y=1, width =3, height=5
Rectangle 3 and Rectangle 2 intersect to form x=3, y=3, width =2, height=2
Rectangle 3 and Rectangle 5 intersect to form x=3, y=3, width =1, height=2
Rectangle 5 and Rectangle 2 intersect to form x=1, y=1, width =3, height=2
Bounding box: x=0, y=0, width =6, height=10


Repeal Obamacare.
http://www.dontfundobamacare.com/
http://liberty-amendments.freeforums.net/
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

Well, I haven't looked at all of your code (there's rather a lot) but the first thing I would do would be to get rid of lines 5 and 6 in the posted code. You don't need to store anything but the location of one corner and the width and the height.

And frankly I would make a Rectangle object immutable, i.e. I wouldn't have any setter methods at all. I would just have a constructor which accepted x, y, width, and height and a set of suitable getters, but no setters. This would make questions like "why is the height changing?" go away.
Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

Paul Clapham wrote:Well, I haven't looked at all of your code (there's rather a lot) but the first thing I would do would be to get rid of lines 5 and 6 in the posted code. You don't need to store anything but the location of one corner and the width and the height.

And frankly I would make a Rectangle object immutable, i.e. I wouldn't have any setter methods at all. I would just have a constructor which accepted x, y, width, and height and a set of suitable getters, but no setters. This would make questions like "why is the height changing?" go away.


The height isn't changing in a Rectangle that's being created so much as it is returning different things in my intersect(), and who knows, maybe in my union method too.

I DO need the upper right corner. That's a vital part to doing the assignment.

The thing is, that normally I could derive the upper right corner's x and y from the given width and height and given the lower left corner. However, when I'm returning a Rectangle, say in intersect, I have to figure out the lower left and upper right corners. From those, I can get the height and width. However, since I can't get the height outright, my getUpperX() and getUpperY() had a slight problem. Sometimes I could get the upperX and upperY from the parameters given to the Rectangle and,unless I used the variable name instead of a get method, though I'm currently trying the variable name instead of the get method and it's better, but I have intersect A and B returning a slightly different rectangle, same point and same width but a slightly different height.

I'm working on the whole immutable thing. May be difficult with the upperX and upperY part though as there is two different ways of calculating it, unless it explicitly set it in the constructor. (Maybe I should do that.)

Nope, the problem is still there.

It's not that big of a deal. One of them has got to be correct.

It's a bit shorter in code now, though I'm posting the whole thing, though not all is needed.

I have removed ALL the setter methods.



Tester class:



All other parts of the thing are working fine (i.e. the union, the bounding box, the overlap counter. Detecting WHICH rectangles intersect, the x, y, and width of the intersecting rectangles. Only the height of the intersecting rectangles appears to be faulty.)

It's possible that my union is also going to produce faulty ones, though all the test cases I've had on it for the bounding box are coming out right.


Christophe Verré
Sheriff

Joined: Nov 24, 2005
Posts: 14687
    
  16

I haven't been looking at your problem, just at the code, and I see at least one issue. You forgot to set upperX and upperY in "public Rectangle(Rectangle a)". To avoid that, you should make one initialize method and call it from both constructors.


[My Blog]
All roads lead to JavaRanch
Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

Christophe Verré wrote:I haven't been looking at your problem, just at the code, and I see at least one issue. You forgot to set upperX and upperY in "public Rectangle(Rectangle a)". To avoid that, you should make one initialize method and call it from both constructors.


Actually I made some of those for fun, though I needed some of them, like the default constructor, later.

However, now all of them have no more setter methods. But the problem still persists. It could be how it's calculating it.

A intersect B != B intersect A even though it should be.

I'm not sure about union either, but the bounding box, which involves a bunch of union calls, is coming out right I think.


Also, when you say immutable, does that mean it CAN'T ever be changed?

For instance

String str = "Am I immutable?";

str = "Or am I not?";

I have heard that Strings are immutable but doesn't my example above contradict that in some way?

I now get that immutable means it has no getter or setters to change it. (Before I had thought it meant that it was in some way akin to making it final.)


However, now I think I understand what it means by "immutable". It means that, outside the class, you can't reset variables. You might be able to reassign a class object but not a variable of the class.

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

Paul Adcock wrote:However, now I think I understand what it means by "immutable". It means that, outside the class, you can't reset variables. You might be able to reassign a class object but not a variable of the class.


Yes, that's correct. Once you have created a Rectangle at (6, 33) with a width of 137 and a height of 42, you can't ever change those values if the class is immutable. Which, I think, would be a good thing.

As for the upper-right corner, I still don't think you need to store its x-y coordinates in the object. You already had perfectly good getter methods for it, which did the simple calculations.
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7536
    
  18

Paul Adcock wrote:I was trying to make a Rectangle class...

Paul,

Could you please re-read the UseCodeTags page again? And when you've done that, perhaps you could break up some of those enormous lines inside the tags; they screw up the windowing here. I'd do it for you, but they're all over the place. Just use the 'Edit' key to change your posts.

Personally, I can hardly read this thread, and I suspect you're missing out on lots of answers as a result.

Winston


Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 37980
    
  22
Please also get rid of most of those blank lines which make the code harder to read. Also add some {} in future releases, as described here. That makes the code easier to read. People faced with several pages of illegible code will simply ignore it.
Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

Winston Gutkowski wrote:
Paul Adcock wrote:I was trying to make a Rectangle class...

Paul,

Could you please re-read the UseCodeTags page again? And when you've done that, perhaps you could break up some of those enormous lines inside the tags; they screw up the windowing here. I'd do it for you, but they're all over the place. Just use the 'Edit' key to change your posts.

Personally, I can hardly read this thread, and I suspect you're missing out on lots of answers as a result.

Winston


It is still stretched out though I've edited the most recent section of code. If I keep breaking longer lines down, ti will soon make those harder to follow as well.

I've gotten rid of a lot of the extra spaces and shrunk the lines a bit but still it's stretched.

The funny thing is, on my IDE, it's lined up neatly.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

So where are we at now? After all that refactoring is there still a problem? If so, then what is it?

(By the way if you made your Rectangle class immutable, you don't need a method to clone a Rectangle any more.)
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 37980
    
  22
No, long lines are harder to read than broken lines. Read this about line length. Apart from the missing {} that code formatting doesn’t look bad now.
Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

The clone method is mainly just for show, as is the copy constructor.

However, why does making it immutable remove the need for a clone method?

Also, my problem is that A intersect B is returning a nearly exact Rectangle as B intersect A but that the heights are slightly different for the two, even though they should be exactly the same.

As to what is causing this, I have no clue.

A side note is that it seems that if the two Rectangles do completely overlap, then it will return the same Rectangle for A intersect B and B intersect A.

Sometimes it's not showing the intersection despite the overlaps. Though it could be that the intersection Rectangle has a width or height of 0, and I told it not to count those, though they would count as overlaps.

It's all a mystery to me.

In my overlaps method, I added a thing saying to return true if the two Rectangles are equal and now it appears that it's working better. As for why

0 0 1 1
1 1 1 1
2 2 1 1
3 3 11

have overlaps but no intersections, now I realize that that is because they only are meeting at a single point or line, and therefore don't count as they have a width and/or height of 0.

I may have just somehow solved the problem.

Paul Adcock
Ranch Hand

Joined: Jan 22, 2011
Posts: 48

Campbell Ritchie wrote:No, long lines are harder to read than broken lines. Read this about line length. Apart from the missing {} that code formatting doesn’t look bad now.


What missing {}? When should I use that?

Also, the problem is still there, though it's a bit better.








The height of A intersect B is sometimes 1 or 2 off from B intersect A.

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

Missing {}? Look at this code:



We all know what that does. But let's suppose that you decide you need some debugging, so you insert a line:



Now your code is broken because it returns true regardless of whether "equals(a)" is true or not. Because now it's missing a {} pair:



Which wouldn't have happened if you had had the {} pair in the first place:



In other words, you should always write "if (x) { ... }" regardless of whether there's only one statement in the "..." part. Same for "for (...) { ... }" and "while (x) { ... }" and probably some other constructs I haven't thought of.

Anyway we're getting diverted from the bug fixes. So, you haven't posted anything which can be replicated. Don't use a test program which asks the user for information, write one which hard-codes some test cases and then checks whether the results are what you expect. Then you can show us the test case and ask why that particular test case doesn't produce the expected result.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 37980
    
  22
And it is often poor style to writeIf you are following that with else return false; you should simply writeThere are a few more examples here.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Can't find bug in my Rectangle class. Think it might be the fault of my setup.
 
Similar Threads
Array of objects/ graphics painting help
A Quadrilateral extend program
how to call object in another class?
Problem when repainting rectangles