if function works flawless in one case

Question

1.00/5 (1 vote)

See more:

making hash compare utility and I am using an if statement to compare a generated hash to a list of hashes held in a text file. I works flawlessly with the exception of not adding "false" to the listview as a subitem, a blank " " implies false, that the items do not match, which is what I am trying to avoid.

Flawless

C#

private void compareToolStripMenuItem_Click(object sender, EventArgs e)
       {
           try
           {
               foreach (string hashEntries in File.ReadAllLines(dlgFile.FileName))
               {
                   if (hashEntries.Contains(lvSums.SelectedItems[0].SubItems[1].Text) == true)
                   {
                       lvSums.SelectedItems[0].SubItems.Add("true");
                       break;
                   }


               }
           }
           catch (Exception ex) { }
       }

But when I add the else if, so that the "false" subitem appears, then all subitems but the first one appear false.

Flawed

C#

private void compareToolStripMenuItem_Click(object sender, EventArgs e)
       {
           try
           {
               foreach (string hashEntries in File.ReadAllLines(dlgFile.FileName))
               {
                   if (hashEntries.Contains(lvSums.SelectedItems[0].SubItems[1].Text) == true)
                   {
                       lvSums.SelectedItems[0].SubItems.Add("true");
                       break;
                   }
                   else if (hashEntries.Contains(lvSums.SelectedItems[0].SubItems[1].Text) != true)
                   {
                       lvSums.SelectedItems[0].SubItems.Add("false");
                       break;
                   }


               }
           }
           catch (Exception ex) { }
       }

Posted 31-May-15 13:22pm

Herboren

Add a Solution

Comments

Herboren 31-May-15 19:35pm

It comparing other entries in the file to the selected text which is why it is coming back false, even though it exists.

gggustafson 31-May-15 22:35pm

Let me get this right. First you are asking if hashEntries contains the text lvSums.SelectedItems[0].SubItems[1].Text. Contains returns a bool so you do not need (in C#) the == true and != true. The else if should just be else. What's the foreach for? You only execute it once because you break on each side of the if/else.

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sergey Alexandrovich Kryukov · Accepted Answer · 2015-05-31T16:51:00

Not only "flawed", but also "flawless" fragment is so wrong that it's really awkward to explain it. First of all, as Contains returns Boolean (and not nullable Boolean?), the is no point comparing it with true or false:

C#

bool something = //... no matter what

//...

// silliest thing:
if (something == true) { /* ... */ }

// should be:
if (something) { /* ... */ }

The second think revealing having no clue on how programming is done is "else if". The code is equivalent to

C#

if (hashEntries.Contains(someText))
   // one thing
else if (!hashEntries.Contains(someText))
   // another thing

The problem here is the second check: "else if" should be replaced with "if". By the way, it could be not the same if some function or property calculated twice could have some side effect which could make the result of the second call different from first one, but of course this is not the case.

Now, worst thing: both fragments of "code" are incorrect just because it can throw unwanted exception. Say, lvSums.SelectedItems[0] is not necessarily successful, because the number of selected items could be null. If it is successful, lvSums.SelectedItems[0].SubItems[1] is not necessarily successful, because number of sub-items can be less than 2.

Further detail could be found through the use of the designer. Such questions generally make little to know sense, but I hope my answer can help to understand what is really flawed and what you need to learn. The flawed is your very basic coding skills, and what you have to learn is: programming requires using some brain and understanding each like you write.

—SA

Herboren · Accepted Answer · 2015-05-31T18:50:00

Took both SA and Gustafson advice. Thank you guys, The code below appears to work

C#

private void compareToolStripMenuItem_Click(object sender, EventArgs e)
        {
            string[] hashEntries = File.ReadAllLines(dlgFile.FileName);

            try
            {
                if (lvSums.SelectedItems.Count > 0)
                {// Filename not null
                    if (lvSums.SelectedItems[0].Text != null &&    
                       // Generated Hash not null         
                        lvSums.SelectedItems[0].SubItems[1].Text != null && 
                        // File Size not null    
                        lvSums.SelectedItems[0].SubItems[2].Text != null)   
                    { // Column four for expected boolean                                                      
                            if                          (hashEntries.Contains(lvSums.SelectedItems[0].SubItems[1].Text))
                                lvSums.SelectedItems[0].SubItems.Add("true");
                            if (!hashEntries.Contains(lvSums.SelectedItems[0].SubItems[1].Text))
                                lvSums.SelectedItems[0].SubItems.Add("false");
                    }
                }           
            }
            catch (Exception ex) 
            { 
                MessageBox.Show("Exception: " + ex.ToString(), "Exception Thrown", MessageBoxButtons.OK,MessageBoxIcon.Error);   
            }
        }

if function works flawless in one case

2 solutions

Solution 1

Solution 2

Add your solution here

Preview 0