I'm looking forward to some tips on how to improve this code I wrote. Don't rewrite it yourself please, as I'm really not looking for someone else to do my homework. What advice would you give me?

import java.util.Random;


public class CompareTo {

	public static void main(String[] args) {
		int x;
		int y;
		
		int[] ints = new int[5];
		
		boolean b1 = false;
		boolean b2 = false;

		Random r = new Random();
		
		if (args.length < 2) {
			System.err.println("You must enter 2 integers.");
			System.exit(0);
		}
		
		for (int i = 0; i < ints.length; i++) {
			ints[i] = r.nextInt(4);
		}
	

		try {
			x = Integer.parseInt(args[0]);
			y = Integer.parseInt(args[1]);
			
			for (int i = 0; i < ints.length; i++) {
				if (x == ints[i]) {
					b1 = true;
				} 
				
				if (y == ints[i]) {
					b2 = true;
				}
			}
		
		} catch (NumberFormatException n) {
			System.err.println("Not a number.");
			System.exit(1);
		}
		
		if (b1 && b2) {
			System.out.println("Found a match.");
		}
	}	
}

Thank you.

Recommended Answers

All 7 Replies

Can you document what the code is supposed to do?
How can anyone comment on whether the code does what it is supposed to do, if there is no statement saying the code is to do .....

The code searches for a match for two numbers from the user x, y in an array of 5 random generated numbers. If both numbers are found it displays a message, else nothing.

The code works fine, I want to know if the approach is correct.

Can x = y?
If not, no need to test for y == if x was == (ie use else if for y test)

The error message doesn't show the invalid number

Why put the search loop inside of the try catch. Its not related.

You could exit the search loop as soon as both numbers were found vs continuing the search.

What if x or y is greater than the max number in the array (3)?

Can x = y?
If not, no need to test for y == if x was == (ie use else if for y test)

I don't need an else for that statement because I want both of them to be present in the array.

What if x or y is greater than the max number in the array (3)?

Then, obviously there won't be a match.

Why put the search loop inside of the try catch. Its not related.

Well the for loop inside depend on x and y.

You could exit the search loop as soon as both numbers were found vs continuing the search.

I could use a nested for loop for that, would that be more efficient?

I don't understand your argument against the else. If the first if is true, the second one can NOT be true. So why test?

If x or y is greater than the max value possible, the search is a waste of time.

The variables can be defined outside of the try catch. putting the search inside the loop clutters the code.

Exiting as soon as the matches are found would require an extra if test, not another loop.

Ok, I understand what you mean by that. I could of just as well used if ((x == ints) && (y == ints)), it's the same thing.

I moved the for loop outside of the try-catch block. I added extra code to first search for the max value and compare that to x respectively y.

Thank you for the tips, they were very useful.

Some more:
The boolean variable names are poorly chosen. Use self documenting names like:
foundX and foundY vs b1 and b2

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.