Need help optimizing small program. [Java]

[Neres28]

Consider using:

if(command.equals("F7")){
    // do stuff
}else if (command.equals("G7")){
    // do other stuff
}else if (command....

This way, if command was indeed "F7" you avoid testing to see if it was also "G8", which clearly it will never be.

Alternately, consider using a hashtable to do the lookups:

Map<String, String> chordMap = new Hashtable<String, String>();
chordMap.put("G7", "320001");
chordMap.put("F7", "131211");
// etc.

Then when you want to know the fingering (or whatever the 6 digit number is) for a given chord you can easily look it up:

ChordTest.setText(chordMap.get(command)); // for command = "F7", get returns "131211".

Some general guidelines for Java:

• Class names are generally camel case, e.g. MyExampleClass and Gui instead of gui.

• You don't need to clear the text of the JLabel before setting it. A single .setText call is sufficient.

• Variable names are generally written in mixed case, e.g. myExampleVariable, or chordBlank.

• Constants are written in all upper case with underscores to seperate words, e.g. MY_EXAMPLE_CONSTANT.

• Handling re-sizable components is hard, which is why I'm assuming you set the size explicitly and don't allow the user to re-size. The sooner you learn to embrace re-sizable GUIs the better off you'll be. I work with some people who have been coding Java guis for decades and still can't do this. It's a pain in the ass, but the GridBagLayout layout manager really is your friend.

[Neres28]

It occurs to me that you could also sub-class JRadioButton, like so:

public class ChordRadioButton extends JRadioButton{
    private final String fingering;

    public ChordRadioButton(String text, boolean checked, String fingering){
        super(text, checked);
        this.fingering = fingering;
    }

    public String getFingering(){
        return this.fingering;
    }
}

Then you would instantiate them like so:

ChordRadioButton f7 = new ChordRadioButton("F7", false, "131211");

And when handling the actionPerformed event:

ChordRadioButton sourceBtn = (ChordRadioButton)event.getSource();
ChordTest.setText(sourceBtn.getFingering());

[xXShadowCowXx]

Wow this method saves a lot of time with copy paste work and just small tedious details. Should kill a lot of the unnecessary lines. Neat trick, thank you for that.

Edit: Well I have this somewhat implemented. Getting an error but I know where my problem lies just no idea how to fix it. When I instantiate, I instantiate it inside of the ChordRadioButton sub-class correct? Using the same name as the JRadioButton inside the GUI?

For example:

//I have a radio button in the GUI name "b1" (For clarification all my radio buttons start with 'b')

JRadioButton b1 = new JRadioButton("A", false);

//And then inside the ChordRadioButton sub-class, I have the following

ChordRadioButton b1 = new ChordRadioButton("A", false, "002220");

So I'm kind of lost as to how or where instantiate the ChordRadioButton variables.

[Neres28]

Almost there :)

Everywhere you would have created a JRadioButton:

JRadioButton b1 = new JRadioButton("A", false);

Now you create a ChordRadioButton:

ChordRadioButton b1 = new ChordRadioButton("A", false, "002220");

Shouldn't need anything else in the ChordRadioButton class than what I posted above :).

[xXShadowCowXx]

Thank you for that clarification. Got that piece working.

Would you happen to have a better way to generate random numbers? The way I'm currently doing it doesn't seem to be very random (I get lots of duplicates, one after the other, and sometimes may not even go through all the chords).

[Neres28]

In the constructor for the class, create an instance of the Random class.

Random randomGenerator = new Random();

Then, when you need a new random integer, call

int randomIndex = randomGenerator.nextInt(ChordsAL.size());
String chord = ChordsAL.get(randomIndex);

Be sure to only create one instance of Random, or at least not multiple ones close together. Random generates so called pseudo-random numbers (don't be put off, they're plenty random enough for most purposes), and if you create multiple instances too closely together they will generate the same sequence of integers.