trjhtr

Does the way I am storing and printing out the data make any sense? or am I going about it all wrong. the code works great, but I'm not entirely sure it was the best way to go about it. Also, I think I can ditch the arrayList and use an array, but I'm not sure if I should.

import java.util.ArrayList;
public class CatapultTester
 
 public static void main (String args)
 25 Deg 30 Deg 35 Deg 40 Deg 45 Deg 50 Deg ");
 System.out.println("*************************************************************************");

 for(int x = 0, i = 0; x < NumSpeeds; x++)
 
 double mph = cat.get(i).getMilesPerHour();

 System.out.printf(" %-8.0f", mph);

 for(int y = 0; y < NumAngles; y++, i++)
 
 double dist = cat.get(i).getDist();

 System.out.printf("%-10.2f ", dist);
 

 System.out.print("n");
 

 System.out.println("*************************************************************************");
 

Here is the Catapult class

public class Catapult

 private double launchAngle;
 private double milesPerHour;
 private double distance;

 /**
 * Constructor for objects of class Catapult
 * 
 * @param angle The angle at whitch the projectile is launched.
 * 
 * @param mph The speed at whitch the projectile is launched.
 */
 public Catapult(int angle, int mph)
 
 launchAngle = angle;
 milesPerHour = mph;
 

 /**
 * Calulates how far the projectile travels
 */
 public void calcDist()
 
 distance = (Math.pow(milesPerHour, 2) * Math.sin( 2 * Math.toRadians(launchAngle)) / 9.8);
 

 /**
 * @return Returns distance.
 */
 public double getDist()
 
 return distance;
 

 //not used, but I added it anyway. it could be used to make a dynamic header for the output.
 /**
 * @return Returns launchAngle.
 */
 public double getLaunchAngle()
 
 return launchAngle;
 

 /**
 * @return Returns milesPerHour.
 */
 public double getMilesPerHour()
 
 return milesPerHour;
 

Also, sorry about the messy code.

Welcome to Code Review! You did a good job so far, but I have some style and logic suggestions for you.

Let's talk about the Catapult class first. In its current form is a good container but it has two flaws. First, you declare the constructor to accept int parameters but are storing them in double variables. Either it should be all int or should be all double, but I am assuming for the rest of this answer that you want double. The second flaw is that you calculate the distance in a separate function and not the constructor. This concept of a cached value is not the flaw, but the way you are using it, is. The only reason you would want to calculate that distance in a different location than in the constructor of Catapult is if it was a very expensive operation that might not need to be called. For this simple example, make the function private and call it in the constructor as follows:

private double angle;
private double speed;
private double distance;

// The second parameter is named "speed" to be consistent with the rest of the
// code. Its better to write a comment or documentation stating that "angle"
// is in degrees and "speed" is in miles per hour.
public Catapult(double angle, double speed) 
 this.angle = angle;
 this.speed = speed;
 this.distance = this.calculateDistance();


...

private double calculateDistance() 
 return speed * speed * Math.sin(2 * Math.toRadians(angle)) / 9.8;


public double getDistance() 
 return this.distance;

Also, rename the other getters to getAngle and getSpeed accordingly.

(Edit: perhaps getLaunchAngle can stay, but getMilesPerHour is not a good function name.)

Next let's talk about CatapultTester. You should move the constants that are defined at the top of main into the class definition itself. I suggest making a constructor for a CatapultTester that takes in those arguments. This way, you have the ability to make several unique CatapultTester objects. If, for example, you want the user of your library to test the Catapult in a custom way, they can build their own CatapultTester objects. Provide that ability to the user:

public class CatapultTester 
 // make sure these are reasonable defaults
 private double startAngle = 25;
 private double startSpeed = 20;
 private double angleInterval = 5;
 private double speedInterval = 5;
 private double numAngles = 6;
 private double numSpeeds = 7;

 public CatapultTester() 
 public CatapultTester(double startAngle, double startSpeed,
 double angleInterval, double speedInterval,
 double numAngles, double numSpeeds)
 
 ... // implement custom constructor here
 

 public void printTest() 
 ... // put your test program here instead
 

 ... // add more functionality in the future?

The point of doing this is that you're putting all the details of the object in the class itself instead of the main program. The main function would end up looking something like this:

public static void main(String args) 
 new CatapultTester().printTest();

This clearly conveys the meaning behind your main function, without the specifics that don't need to be known by the user. Now, if you want to expand main in any way, you have a more powerful API to work with.

Now to your actual test function. There are several instances of duplicated code, note the three for loops that run over the same range. What you should do instead is just loop over the range you need and do all the computation in one step. This would also remove the need for an array for the Catapult objects.

The last for loop is enough:

System.out.println(" | Projectile Distance (ft.) ");
System.out.println("*************************************************************************");
System.out.println(" MPH | 25 Deg 30 Deg 35 Deg 40 Deg 45 Deg 50 Deg ");
System.out.println("*************************************************************************");

for (int speed = 0; speed < this.numSpeeds; ++speed) 
 StringBuilder output = new StringBuilder(); // StringBuilder is fast!
 for (int angle = 0; angle < this.numAngles; ++angle) 

 // build one catapult at a time and calculate all necessary values in one go
 Catapult cat = new Catapult(
 angle * this.angleInterval + this.startAngle,
 speed * this.speedInterval + this.startSpeed
 );

 // prints only once at the beginning of the angle loop
 if (angle == 0) 
 output.append(String.format(" %-8.0f", cat.getSpeed()));
 

 output.append(String.format("%-10.2f ", cat.getDistance()));
 

 System.out.println(output.toString());


System.out.println("*************************************************************************");

You will have to import java.util.StringBuilder for this functionality. This drastically reduces the number of computations and repetitions in your code. Its important to keep in mind the acronym DRY (Don't Repeat Yourself) and if you see something that happens over and over again, see if you can write it once but get the same computations out of it. It will speed up your code and make it more readable. I hope you found these suggestions helpful. Good job and good luck!

 System.out.println(" | Projectile Distance (ft.) ");

Maybe I'm missing something, but where's the conversion to feet? I see a calculation using a speed in miles per hour and an acceleration in metres per second squared. It would be an astonishing coincidence if that gave a distance in feet without an extra conversion factor.

 distance = (Math.pow(milesPerHour, 2) * Math.sin( 2 * Math.toRadians(launchAngle)) / 9.8);

Magic number alert! The acceleration due to gravity should be pulled out into a constant (static final field).

Where does this formula come from? What limitations does it have?

There are two big limitations:



1. It assumes no air resistance. If you want to calculate values for practical use (e.g. a realistic game) you need to use air resistance, and then you don't get a nice formula but have to do quadrature. If you look hard enough it's possible to find data on historical cannons' shell weights, muzzle velocities, and maximum ranges; and from that data and an assumption that air resistance has a linear and a quadratic component in the speed of the shell it's possible to estimate their coefficients and interpolate/extrapolate. But it's 15 years since I did this, and I don't still have the values to hand.



2. It assumes that the ranges are short enough that the curvature of the Earth is irrelevant. (This is realistic: ICBMs are not launched from a cannon).

Both of these issues can be finessed by limiting the muzzle velocity severely, but the constructor doesn't place any limits on muzzle velocity. In fact, it doesn't even complain if it's negative! I think an IllegalArgumentException would be appropriate in that case.

But just to make it clear: the main point here is that the calculation would benefit from some comments to explain the derivation (or point to a reference) and the limitations / assumptions.