Please help me with improving my code

[ken luong]

Hello guys. Please help to improve my code and logic for the following. I just started learning programming. I hope my inexperience doesn't make you want to scream out loud as you read my code

/* Program to generate a 3 digit lottery number.
 *  Prompt user to enter a 3 digit number 
 *  Determine whether  user's number the same as the generated number, if not
 *  determine the number of matching digits
 */

import java.util.Scanner;
import java.util.Random;

public class Main {
	public static void main(String[] args) {
	
	int winningNumber = generateWinningNumber();
	int playerNumber = getPlayerNumber();
	displayAwards(winningNumber,playerNumber);
	}
	
	public static int generateWinningNumber() {	
		Random random = new Random();
		return random.nextInt(1000);
	}
	
	public static int getPlayerNumber() {
		Scanner input = new Scanner(System.in);
		System.out.print("Enter a your 3 digit number: ");
		return input.nextInt();
	}
	
	public static void displayAwards(int winningNumber, int playerNumber) {
		
		int counter = 0;
		String winningNumberString = Integer.toString(winningNumber);
		String playerNumberString = Integer.toString(playerNumber);
		char [] playerNumberArray = playerNumberString.toCharArray();
		
		for( int i = 0; i < playerNumberArray.length; i ++) {
			if (!(winningNumberString.indexOf(playerNumberArray[i]) == -1)) 
				counter++;
			}
		
		System.out.printf("Winning nuber is %d\n",winningNumber);
		if ( winningNumber == playerNumber)
			System.out.println("jackpot");
		else if (counter == 3)
			System.out.println("3 matches");
		else if (counter == 2)
			System.out.println("2 matches");
		else if ( counter == 1)
			System.out.println("1 match");
		else
			System.out.println("Better luck next time");
			
	}
}

[Campbell Ritchie]

Have you been taught to make everything static? I hope not; that isn't how the language is designed. Move all your “real” code out of the main() method.
Don't use the same object operator == on reference types (line 42). You can rely on that to produce errors.
Why are you asking for three digits and then printing jackpot/three digits right (lines 42‑46)?
I don't think you need that block of if‑elses.
Avoid multiple Scanner objects. You only need one Scanner object to read from System.in.

[Campbell Ritchie]

What happens if the player enters the same digit repeatedly, e.g. 000, 111 100, 666?

[Monica Shiralkar]

Campbell Ritchie wrote:Have you been taught to make everything static? I hope not;

That's a problem for me too regarding how I am using static where I shouldn't  .Unfortunately I have got into a bad way of coding where I move out code from my main method into new methods but then move all of these into a Helper class making them static and call from the original class. As you said that's not a good way to make each method static.

[Campbell Ritchie]

There is nothing wrong with helper classes; they are useful things as long as they don't try to abstract instance data away. Helper classes are probably the same as utility classes. Becaus they don't contain any instance data, they usually have all methods static. I shall let you explain, MS, why they should also have a private constructor.

[Junilu Lacar]

Some questions to spur thought and discussion:

1. What's wrong with static methods?

2. When is a static method appropriate?

3. When is it more appropriate to use an instance method instead?

[Junilu Lacar]

The positive:

1. Main method is short and fairly self-explanatory

2. Different concerns are separated - get random number, get user number, check how many digits match

Can be improved:

1. Names have types included in them - playerNumberString, for example.  This is poor form. In general, names should reflect purpose/intent, not the implementation.

2. Some smaller concerns can be separated out further. The first indicator is that the displayAwards() method is quite long. There are at least two concerns mixed together in that method: calculating how many digits match and displaying the results.

[Monica Shiralkar]

Campbell Ritchie wrote:There is nothing wrong with helper classes; they are useful things as long as they don't try to abstract instance data away. Helper classes are probably the same as utility classes. Becaus they don't contain any instance data, they usually have all methods static. I shall let you explain, MS, why they should also have a private constructor.

Yes but this way I never create non static methods which certainly looks wrong.


For example whatever I have to do in my class , I first write in main method and then try to break into smaller methods.I move them to separate methods obviously static (as static main can call only static ).

The problem with this style of mine is not that I am creating static methods and moving to Helper class but that thus I am never creating normal methods for any work given to me.Something is surely wrong .

[Junilu Lacar]

Monica Shiralkar wrote:Something is surely wrong .

The question is, why do you think it's wrong? What principle/concept are you violating? You have to understand why before you can fix the what.

[Monica Shiralkar]

Junilu Lacar wrote:

Monica Shiralkar wrote:Something is surely wrong .

The question is, why do you think it's wrong? What principle/concept are you violating? You have to understand why before you can fix the what.

I am thinking it's wrong because , in Java like everything normal methods too exist for some reason but my way of coding never uses them.

[Junilu Lacar]

Monica Shiralkar wrote:I am thinking it's wrong because , in Java like everything normal methods too exist for some reason but my way of coding never uses them.

And what reason do you think that is? You need to understand that reason, you can't just say "for some reason."

[Monica Shiralkar]

I think that since normal methods are used for changing the value of the variables of the class ,if I am just doing some other processing then static is fine.

[Junilu Lacar]

Monica Shiralkar wrote:I think that since normal methods are used for changing the value of the variables of the class.

You should be very precise in your language, especially when discussing things like this. What exactly do you mean by "the value of the variables of the class"? There are two types: class members and instance members. Which one do you mean?

[Monica Shiralkar]

Sorry, I meant class members.

[Junilu Lacar]

Monica Shiralkar wrote:Sorry, I meant class members.

You wrote "normal methods are used for changing the value of the variables of the class" and clarified that you meant "class variables." I assume that by "normal methods" you also meant instance methods. Your entire statement is wrong then.

[Monica Shiralkar]

Instance methods are used for modifying the value of the class variables.

[ken luong]

Campbell Ritchie

Don't use the same object operator == on reference types (line 42)

What happens if the player enters the same digit repeatedly, e.g. 000, 111 100, 666?

The usage of the == operator compares the reference variables rather than the content of the objects which are referenced by the reference variables. Let's consider s1 and s2, which are String objects created dynamically. Suppose that s1 = "hi" and s2 = "hi". In this scenario, s1 == s2 would returns false, since s1 and s2 are different variables. String.equals() should be used instead of ==. This is what i gathered after some reading.


If the winning number is 123 and the user enters 111, counter would be 3, which is wrong. I can see the logic error. Thanks. Campbell Richie!

[Junilu Lacar]

Monica Shiralkar wrote:Instance methods are used for modifying the value of the class variables.

Absolutely not.

[Junilu Lacar]

if ( winningNumber == playerNumber)

There's nothing wrong with using == on line 42. Both variables are primitive int values.

[Monica Shiralkar]

Junilu Lacar wrote:

Monica Shiralkar wrote:Instance methods are used for modifying the value of the class variables.

Absolutely not.

But for e.g in case of POJO Classes aren't we setting (changing ) the class variables and getting  them using instance methods (getters and setters ).

[Junilu Lacar]

I think you need to review your object-oriented concepts. Instance variables define the state of an object. Each object has its own state and instance methods should operate on that state. Class variables apply to all objects of the same class. The fact that you can access class variables from instance methods doesn't mean that you should. In general, however, doing that is something you should be very suspicious about.

[ken luong]

When to declare variables and methods static?

Variables and methods of a class should be declared as static if they are independent of any specific object. In other words, if variables and methods are of class level, they should be declared as static.
Static methods only have access to static member variables and can't be overriden in derived classes.
Static methods can't invoke member functions but member functions can invoke static methods and static member variables.
Static methods and variables can be accessed via class name as well as objects.

Please add to my understanding of static, Thanks!

[Campbell Ritchie]

ken luong wrote:. . . s1 = "hi" and s2 = "hi". In this scenario, s1 == s2 would returns false . . .

Not necessarily. I advise against working out why at this stage of your career, but it is another reason for not using ==.

[Campbell Ritchie]

Monica Shiralkar wrote:Sorry, I meant class members.

Do you remember what the official definition of class members is? It's in the Java® Language Specification.

[Campbell Ritchie]

Things static belong to the class, and are independent of any objects.

[Monica Shiralkar]

Monica Shiralkar wrote:Instance methods are used for modifying the value of the class variables.

I did mistake due to my incorrect understanding of naming. Sorry, my mistake lead to confusion in this thread. I was thinking that instance variables are called class variables but instead static ones are called class variables.
So instead I had meant to say that instance methods are used for accessing and modifying instance variables .E.g POJO class.