CODEWITHSUNDEEP
javaperformancedebuggingmethodsconditional-statements
user2707813
user2707813

Reputation:

Why isn't my java code proceeding past my if statements?

Okay, I have never had this problem before so I am not sure how to word it or to repair it, I am building a java application that creates dealers, in that application I have the parameters passed in to the DealerFactory.createDealer method and proceed to first check if that dealer exists with a conditional statement that looks like this:

    if (DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId)) {

        throw new Exception("Sorry That Dealer Already Exists");

    } else if (DealerFactory.fetchDealer(loginId).getId().equals(DNo)){

        throw new Exception("Sorry That Dealer Already Exists");

    } else {
         << proceed with the rest of the method here >>

I have seen this done before so as to check the availability of the username and the id of the person being created. However after running it I found that if I create the dealer and the condition evaluates to true the if statement works just fine letting me know that I have created a user that already exists and I need to create him/her with new a different Id and username. However if the condition evaluates to false I never seem to make it into the else portion of the statement, I am getting no errors, no compilation issues, and no exceptions I have written the statement differently to try that, which wasn't really any different it just looked syntactically different:

 if (DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId)
            || DealerFactory.fetchDealer(loginId).getId().equals(DNo)) {

        throw new Exception("Sorry That Dealer Already Exists");
    }

I have included println statements to follow the program through its run and when the condition evaluates to false I never make it into the else statement. I cant seem to figure out why it is breaking on the condition statement when it is evaluated to false, any thoughts?

edit:::

Ok so that I can be of more assistance in helping you guys help me lol here is the method in its entirety, I apologize for not posting it in the first place

public static int create(String DNo, String name, String admin,
        String loginId, String password, String callSrc, String voiSys,
        String whoCall, String callBrt, String active, String adfEmail)
        throws SQLException, Exception {

    int validateResult = 0;


    if (DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId)
            || DealerFactory.fetchDealer(loginId).getId().equals(DNo)) {

        throw new Exception("Sorry That Dealer Already Exists");
    }

        try {

            DealerFactory.pool = DBConnector.getInstance();
            DealerFactory.connect = DBConnector.getConnection();
            DealerFactory.preparedStatement = connect
                    .prepareStatement("Insert Into Dealers (DNo, Dealer, Admin, Login, Password, CallSrc, VoiSys, WhoC, CBrt, Active, ADFemail) "
                            + "values(?,?,?,?,?,?,?,?,?,?,?)");
            DealerFactory.preparedStatement.setString(1, DNo);
            DealerFactory.preparedStatement.setString(2, name);
            DealerFactory.preparedStatement.setString(3, admin);
            DealerFactory.preparedStatement.setString(4, loginId);
            DealerFactory.preparedStatement.setString(5, password);
            DealerFactory.preparedStatement.setString(6, callSrc);
            DealerFactory.preparedStatement.setString(7, voiSys);
            DealerFactory.preparedStatement.setString(8, whoCall);
            DealerFactory.preparedStatement.setString(9, callBrt);
            DealerFactory.preparedStatement.setString(10, active);
            DealerFactory.preparedStatement.setString(11, adfEmail);

            validateResult = DealerFactory.preparedStatement
                    .executeUpdate();

        } catch (SQLException ex) {

            System.err.println("Error: " + ex + "\n");
            ex.printStackTrace();

        } finally {

            DBUtils.closePrepStatement(DealerFactory.preparedStatement);
            DealerFactory.pool.freeConnection(DealerFactory.connect);

        }

    return validateResult;
}

Upvotes: 1

Views: 168

Answers (2)

raisedandglazed
raisedandglazed

Reputation: 824

More information regrading the variables that you are passing into the conditional would be of great help, (i.e. loginId and DNo). The first thing I would do is simplify your conditions in the if & else if statements.

It seems like you could change the if conditional to be if(DealerFactory.fetchDealer(loginId)) and have it work just the same since if the factory returns a dealer with the ID that you are looking for, that dealer already exists. From my understanding of what you're trying to achieve, it's unnecessary to dig any deeper. Also try running through this section of code with the debugger and monitor variables step-by-step to see what things look like at each line of code.

Edit:

You could also move the expressions outside of the conditional and set them to variables. For example:

int dealerId =  DealerFactory.fetchDealer(loginId);
if(dealerId != null && dealerId != loginId) {
      //<< proceed with the rest of the method here >>
} else {
     //<<throw exception>>
}

Checking to see if dealerId is null in the conditional would avoid NullPointerExceptions, and declaring things outside of the conditional will make it easier to debug by hand.

Upvotes: -1

nem035
nem035

Reputation: 35491

First things first, you shouldn't chain methods like that due to dangers of NullPointerException

So this part:

if(DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId))

Could look something like this:

if(DealerFactory.fetchDealer(loginId) != null && 
   DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId))

Or you could have a separate null check before all your if statements.

However, what you are doing is an overkill. This whole part:

DealerFactory.fetchDealer(loginId).getLoginId()

Returns null if you cannot find a dealer or loginId that you already have. Assuming your fetchDealer() method returns null if dealer cannot be found.

Instead of:

if(DealerFactory.fetchDealer(loginId).getLoginId().equals(loginId)))

You can just do:

if(DealerFactory.fetchDealer(loginId) != null)

Another improvement you could do is to add a method to DealerFactory called dealerExists(String id) declared something like this:

boolean dealerExists(String id) {
    return (YOUR_DATA_STRUCTURE_OF_DEALERS.get(id) != null);
}

Or even:

boolean dealerExists(String id) {
    return (fetchDealer(loginId) != null);
}

This would allow for better logical flow of your code. Your if statement would be very clear then:

if(DealerFactory.dealerExists(loginId) {
    throw new Exception("Sorry That Dealer Already Exists");
}

Also, the value DNo is the dealer number I presume and you are checking if a dealer exists with the provided loginId or the provided DNo. However, in you checks you are comparing the DNo to the loginId to check if a dealer exists. What exactly is the point of DNo and shouldn't the loginId be enough to determine that the dealer exists? If you really need to check the DNo as well, just add it as a check within the methods I suggested above.

Upvotes: 2

Related Questions