CODEWITHSUNDEEP
c#
Andrew Reese
Andrew Reese

Reputation: 912

C# if statement always returning false

I have an if statement in a web service call that is always returning false. I want it to return an error message if there isn't the words "VERTICALPALLETS", or "PALLETLABELS" in it. Even if this field has those words in it, it is still returning an error. The query is not the correct one I am using in my application. Any help is much appreciated. 

public bool ValidateDevice(string DeviceID, string sVersion, out string MachineID, out string MachineName, out string Plant, out string Printer1, out string Printer2, out string WrapStation, out string Location, out string sMessage)
{
    MachineID = "";
    MachineName = "";
    Plant = "";
    Printer1 = "";
    Printer2 = "";
    Plant = "";
    WrapStation = "";
    Location = "";
    sMessage = "";


    bool bTest = CheckVersion(sVersion, out sMessage);
    if (bTest == false)
    {
        sMessage = "You do not meet the minimum version requirements. Contact MIS.";
        return false;
    }


    try
    {
        SqlConnection connection = new SqlConnection(capmSADPConnectionString);
        connection.Open();

        string queryString = "select * from DATABASE";
        SqlCommand command = new SqlCommand(queryString, connection);
        command.Parameters.Clear();
        command.Parameters.AddWithValue("@DEVICEID", DeviceID);
        command.Parameters.AddWithValue("@APPID", "VTP");

        SqlDataReader reader = command.ExecuteReader();

        if (reader.HasRows == false)
        {
            email myEmail = new email();
            myEmail.SendErrorEmail("No Application Settings for VTP Device ID - " + DeviceID, "VTPError", "VTP Device Settings Error");
            sMessage = "Could not find application settings for this device. Please enter settings.";
            return false;
        }




        reader.Read();

        MachineID = reader.GetValue(0).ToString();
        MachineName = reader.GetValue(1).ToString();
        Plant = reader.GetValue(2).ToString();
        Location = reader.GetValue(3).ToString();
        Printer1 = reader.GetValue(4).ToString();
        Printer2 = reader.GetValue(5).ToString();
        WrapStation = reader.GetValue(6).ToString();

        reader.Close();
        reader = null;
        command = null;
        connection.Close();
        connection = null;

        if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) || (Location.ToUpper().Contains("PALLETLABELS") == false))
        {
            sMessage = "Please enter correct location in Application Setup and try again.";
            return false;
        }



        return true;
    }
    catch (Exception e)
    {
        sMessage = "Unable to retrieve Application Settings for VTP. Please reopen the application.";
        return false;
    }
    return true;
}

Upvotes: 0

Views: 1250

Answers (4)

Lajos Arpad
Lajos Arpad

Reputation: 76583

This line

if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) || (Location.ToUpper().Contains("PALLETLABELS") == false))

checks whether Location.ToUpper() does not contain "VERTICALPALLETS" or Location.ToUpper() does not contain "PALLETLABELS". If Location.ToUpper() does not happen to contain both, then the condition is true. You instead wanted to check whether Location.ToUpper() contains either of them:

if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) && (Location.ToUpper().Contains("PALLETLABELS") == false))
{
    //...
}

also, it does not makes much sense to calculate ToUpper several times, let's calculate it only once:

String upperLocation = Location.ToUpper();
if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) && (Location.ToUpper().Contains("PALLETLABELS") == false))
{
    //...
}

Upvotes: 1

Carl Verret
Carl Verret

Reputation: 616

Sounds like the problem is that you'll always be looking for a "false" or "false" condition unless you are looking for a location that contains both VERTICALPALLETS PALLETLABELS

I suggest you try something like

if (! ( condtion1 || condition2) )
 return message

Upvotes: 1

Martin
Martin

Reputation: 16433

The problem here is with the structure of your if statement:

if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) || (Location.ToUpper().Contains("PALLETLABELS") == false))
{
    sMessage = "Please enter correct location in Application Setup and try again.";
    return false;
}

You are saying:

If the location does not contain 'VERTICALPALLETS'
Or Else
If the location does not contain 'PALLETLABELS'

This will always be true unless the location contains both bits of text!

You need to change the Or Else to And Also (so || to &&) and therefore your code will work as expected.

Upvotes: 5

dlxeon
dlxeon

Reputation: 2000

Use AND

if ((Location.ToUpper().Contains("VERTICALPALLETS") == false) && (Location.ToUpper().Contains("PALLETLABELS") == false))

Alternative solution which is simpler to read.

if (!(Location.ToUpper().Contains("VERTICALPALLETS") || Location.ToUpper().Contains("PALLETLABELS"))

Upvotes: 1

Related Questions