• Post Reply Bookmark Topic Watch Topic
  • New Topic

Triangle program. Perimeter problem  RSS feed

 
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Im writing a program that allows users to put in three sides of a triangle for 5 different triangles and then displays the type of type of triangle, perimeter of the triangle and total perimeter of all 5 triangles. The problem Im having is that the calculations for the total perimeter are off. It seems to be adding the perimeter of the last triangle twice. Also my program is supposed to set the values of the sides to 1 if the triangle is not a valid triangle, which it does, but that change is not reflected when displaying the type of triangle it is. Any help would be greatly appreciated! Here is what I have:


 
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Looking at one issue at the time. The total perimeter calculates the perimeter for the 5th traingle twice. Well the method calcPerim() is responsible for this. See how often your calling this method for each triangle.

A tip for your main class testTriangle. Your code will be much cleaner if you get rid of the duplicated codelines by using a loop. Also the main method contains way to much coding. 1. The main should only kick of the process and not contain so much code. 2. Even when moving the code to a method of it's own your best off in splitting this part up in more methods of it's own.

 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In regard to the " incorrect" triangles. Setting their sides to 1 each wil make it a correct traingle in itself. I would consider the following two.
1. Why are you altering the sides of the incorrect traingles. What is the added benefit?
2. You have a method isValid. But you only use it in the Triangle method. Maybe there are more place where it is usefull to use this method..... ;)

Kind Regards.
 
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Calling the addTotalPerim() from the Triangle constructor is not a good approach. A constructor should only be concerned with the current instance; it should not have anything to do with the whole population of instances.

Stated another way, much of object-oriented programming is about assigning responsibilities to an appropriate abstraction. The Triangle constructor should not have the responsibility of helping to track the total perimeter of all Triangles. The responsibility should be given to something other than the Triangle class.

Also, the method names is_right, is_isosceles, etc. do not quite follow the generally accepted naming convention for methods; they should be isRight, isIsosceles, etc. instead.

This code is redundant:

This method and others similar to it can be simplified as:

Finally, Don't Repeat Yourself. You have a lot of duplicated code in your program. you should look for sections of code that look very similar to each other except maybe for a few minor variations. You can save yourself some effort by putting the repeated code in a separate method and using parameters for the parts that vary from one case to another.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One more thing. I suggested that the methods that check the type of Triangle can be simplified as something like

Since this method and others like it are instance members, they really should pertain to the current instance. That is, it is not necessary or even appropriate to pass parameters to these methods. With proper JavaDocs, this method should be like this:
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You can also simplify the isValid method by performing the check for any 0 length sides first, then otherwise return the result of the boolean expression that checks for valid triangle side lengths. Again, this method should pertain only to the current triangle instance so there should be no parameters passed to it.
 
Marshal
Posts: 56605
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

Avoid Math.pow(x, 2). Use x * x instead, which is probably faster. As long as you don't get an overflow error, that is.

To continue from what Julilu said yesterday. Not only should the constructor not record total perimeters, but the Triangle class maybe shouldn't record it either. If you have a list of Triangles, use that List to record the totals.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!