[Darklordshivam]

Hello guys,Ive been debugging my program for a long period of time and cannot figure out whats going wrong.
Here is the main snippet

public void actionPerformed(ActionEvent ae) {
 if (ae.getSource() == Cancel) {
            System.exit(0);
        }

        String user = NameField.getText(); // Username Input by the user
        String password = PassField.getText(); // Password input by the user
               if (user.equals("Shivam") && password.equals(res)) // If user and
                                                            // password match 
        {
            JOptionPane.showMessageDialog(LoginBox,
                    "You Have Been Successfully Logged In !",
                    "Welcome To The Apple Store By Shivam",
                    JOptionPane.PLAIN_MESSAGE);
            Process jf = new Process();
            jf.setSize(400, 300);
            jf.setVisible(true);
        } else {
            JOptionPane.showMessageDialog(LoginBox, "Invalid Login", "Error",
                    JOptionPane.ERROR_MESSAGE);
        }
        if (ae.getSource() == Premium) { //This is another button,if you are a member you enter your code despite of logging in 
            JFrame prem = new JFrame();
        setLayout(new FlowLayout());
        preminfo = new JTextField();
        preminfo.setBounds(25, 90, 380, 30);
        prem.add(preminfo);
        info = preminfo.getText();
        cp = new JButton("OK");
        cp.setBounds(170, 130, 70, 30);
        prem.add(cp);
        cp.addActionListener(this);
        ImagePanel pre = new ImagePanel(
                new ImageIcon(
                        "Mypath\\Premium.png").getImage());
        prem.add(pre);
        prem.setSize(427, 180);
        prem.setVisible(true);
        preminfo.addActionListener(this);
        } //Now the main messed up part
        if (info.equals(pre_pass[0]) || info.equals(pre_pass[1]) //Info is the code entered by the user and pre_pass is the array which stores all the codes
                || info.equals(pre_pass[2]) || info.equals(pre_pass[3])
                || info.equals(pre_pass[4]))// Matches if the input by user is equivalent to those defined in the array
               {
            PremiumProcess jf2 = new PremiumProcess(); //Creates an object of this class defined later
            jf2.setSize(404, 420);
            jf2.setVisible(true);
        } else

            JOptionPane.showMessageDialog(LoginBox, "Invalid Code", "Error",
                    JOptionPane.ERROR_MESSAGE);

    }

It always executes both the else statements,i.e Invalid code and Invalid Login

This post has been edited by Darklordshivam: 13 November 2011 - 09:03 AM

[GregBrannon]

If your 'else' is always executing, your 'if' condition is always evaluating to false. Review your if conditions and determine why they're always false. Write println statements before the 'if' statements to evaluate the if condition variables.

You didn't give us enough info to help you with that, but it's probably better that you do it yourself.

[Darklordshivam]

GregBrannon, on 13 November 2011 - 09:11 AM, said:

If your 'else' is always executing, your 'if' condition is always evaluating to false. Review your if conditions and determine why they're always false. Write println statements before the 'if' statements to evaluate the if condition variables.

You didn't give us enough info to help you with that, but it's probably better that you do it yourself.

My if condition is supposed to be true,Ive crossed checked it thrice.I tried using the debugger but when it reaches the statement where it checks if the button is pressed,it directly exits the whole bunch of statements

[GregBrannon]

Debuggers are great IF you know how to use them. If you're interested in being good at a debugging tool, I suggest using a logger, like log4j. It's not hard to set up and use and the concept is as easy as adding print statements to your code to figure out what's going on.

If your current project's schedule prevents you learning something else right now, add the println statement I suggested to figure out why your thrice-checked if statements aren't evaluating true as you believe they should.

[smohd]

equals() is case sensitive also, so make sure you type exactly Shivam with S in capital, also dont put spaces between. you can try to trim() the input also like:

String user = NameField.getText().trim(();

Another thing to take care, your code for checking is username and password is correct will execute for all events that listen to this actionPerformed() except cancel, so I am not sure why it is out of any if condition to check which sources generates the event.

[Darklordshivam]

smohd, on 13 November 2011 - 09:30 AM, said:

equals() is case sensitive also, so make sure you type exactly Shivam with S in capital, also dont put spaces between. you can try to trim() the input also like:

String user = NameField.getText().trim(();

Another thing to take care, your code for checking is username and password is correct will execute for all events that listen to this actionPerformed() except cancel, so I am not sure why it is out of any if condition to check which sources generates the event.

No,that is not supposed to be an issue,my "S" is capital without any spaces,Ive tried copying pasting too !

[Darklordshivam]

GregBrannon, on 13 November 2011 - 09:27 AM, said:

Debuggers are great IF you know how to use them. If you're interested in being good at a debugging tool, I suggest using a logger, like log4j. It's not hard to set up and use and the concept is as easy as adding print statements to your code to figure out what's going on.

If your current project's schedule prevents you learning something else right now, add the println statement I suggested to figure out why your thrice-checked if statements aren't evaluating true as you believe they should.

Ok I added the print statement,
When I clicked the premium button,It says "Invalid Login" Maybe because I didnt login and/or therefore the username / password were wrong be default,Then it displayed my print statement along with these:
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
at Login.actionPerformed(Login.java:118)
at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1995)
at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2318)
at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:387)
at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:242)
at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:236)
at java.awt.Component.processMouseEvent(Component.java:6267)
at javax.swing.JComponent.processMouseEvent(JComponent.java:3267)
at java.awt.Component.processEvent(Component.java:6032)
at java.awt.Container.processEvent(Container.java:2041)
at java.awt.Component.dispatchEventImpl(Component.java:4630)
at java.awt.Container.dispatchEventImpl(Container.java:2099)
at java.awt.Component.dispatchEvent(Component.java:4460)
at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4577)
at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4238)
at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4168)
at java.awt.Container.dispatchEventImpl(Container.java:2085)
at java.awt.window.dispatchEventImpl(window.java:2478)
at java.awt.Component.dispatchEvent(Component.java:4460)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:599)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

[GregBrannon]

Adding the println statements did all that?

I assume you printed user, password, info and then all of those info comparisons. Are those the variables you printed out, and did you learn anything?

[Darklordshivam]

GregBrannon, on 13 November 2011 - 09:47 AM, said:

Adding the println statements did all that?

I assume you printed user, password, info and then all of those info comparisons. Are those the variables you printed out, and did you learn anything?

Yes I displayed the variable but "info" didnt come up

[jon.kiparsky]

You say both else branches are firing, so let's look at the second one for a moment.

if (info.equals(pre_pass[0]) || info.equals(pre_pass[1])  || info.equals(pre_pass[2]) || info.equals(pre_pass[3])

I'm looking at this and not liking it. Could you sub the checking out to a method?

private boolean checkPrePass(String info) // String? I'm guess here, since I don't have the declaration
{
  boolean returnValue = false;
  for (String s: pre_pass)
  {
    if (info.equals(s)) 
    {
      returnValue=true;
    }
  }
  return returnValue;
}

now you can say

if (checkPrePass(info))
{
  //etc...
}

which will make it easier to see what's going on. This is a minor sort of refactoring, but if you find other such opportunities to simplify your code, it'll be easier to solve this, because you'll be able to look at smaller pieces of code.


You can also use this as a degugging point, by adding a few lines:

private boolean checkPrePass(String info) // String? I'm guess here, since I don't have the declaration
{
  boolean returnValue = false;
  for (String s: pre_pass)
  {

  System.out.println("info: $$"+info+"$$ s: $$"+s+"$$");
  // put distinctive punctuation around the values to make whitespace stand out
    if (info.equals(s)) 
    {
      returnValue=true;
      
    }
  }
  if (!returnValue)
  {

      System.out.println("CheckPrePass returns false");
  }
  return returnValue;
}

As Greg says, logging is a better way to do this, but at this point you have to think about whether the investment of time is worth while. Next time, include the logging from the start, because it'll be easier then.

There are other places you could refactor, as well.

   JOptionPane.showMessageDialog(LoginBox, "Invalid Login", "Error", JOptionPane.ERROR_MESSAGE);

Make a convenience method out of this:

public static void error(String message)
{
   JOptionPane.showMessageDialog(LoginBox, message, "Error", JOptionPane.ERROR_MESSAGE);
}

Easier to read

error("Invalid login);

and you don't have to worry about whether you go all the parameters right each time. I make this one static in my code, so it can appear once, in some convenient class, but you can make it an instance method if you prefer.

And this blob apears at least twice:

   PremiumProcess jf2 = new PremiumProcess(); 
 jf2.setSize(404, 420);
 jf2.setVisible(true);

could be a method called makeProcess


There are many more. By the time you finish refactoring this, you'll probably have fixed the bug without realizing it.