5

Sorry to ask this as I thought I knew the answer, I want to exit the program if userName is greater than 4 characters or userName is not an account called student. However this even if the userName is only 3 characters and is not student I'm still hitting Application.Exit. What am I doing wrong?

if (userName.Length > 4 | userName != "student")
{
    Application.Exit();
}

Shame on me :-(

abatishchev
  • 98,240
  • 88
  • 296
  • 433
Jamie
  • 2,465
  • 10
  • 28
  • 31
  • this statement is meaningless, because you app will not exit only in case userName is `student`. – Andrey May 24 '10 at 13:03
  • What I needed was && as shown below, thanks all! :-) if (userName.Length > 4 && userName != "student") { Application.Exit(); } – Jamie May 24 '10 at 13:05
  • @Jamie, it is again useless. `"student".Length` is > 4 anyway :) – Andrey May 24 '10 at 13:09
  • You could have solved this yourself with some simple use cases, e.g. username = billy-jean => exit, username = student => don't exit, username = jon => don't exit. Clearly you need longer than 4, AND not student. – cjk May 24 '10 at 13:12
  • Also, use userName.Equals("student") instead. – Jason Maskell May 24 '10 at 13:17
  • If this is reflecting some business rule or convention, you should really extract the logic into a separate function with a meaningful name. – Instance Hunter May 24 '10 at 13:26
  • @Mark: I don't see how the `Equals` method is not typesafe? – James May 24 '10 at 13:37
  • @Mark: Ah apologies mis-read your comment. – James May 24 '10 at 13:41

5 Answers5

15

While you should use || instead of |, they will give the same result in this situation. Despite the upvoting of the other answers, changing | to || will not solve your problem.

Your real problem is that the conditions you want to check for will always be true. Either your userName is not student, or it is student and then it is also longer than 4 characters.

When you have a username that is only 3 characters it is not equal to student, therefore the program quits.

From your description of what you expect, I think you mean this:

if (userName.Length > 4 && userName != "student")
{
    Application.Exit();
}
Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
6

You need to use the boolean OR (||) operator instead of the bitwise OR (|)

As I said in my comments though, your logic doesn't necessarily make any sense to me. The way it is writting, the statement will always be true:

  • If userName is not student, the statement is true and the application exits.

  • If userName is student, then length > 4 and the statement is true again (which causes an exit).

You could change things to:

if(username.Length > 4 && userName != "student")
{
    Application.Exit();
}

Which makes more sense logically, but since I don't know your intent I can't guarantee that it would work the way you want it to.

Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
  • `if (userName.Length > 4 || userName != "student")` is meaning less? – Andrey May 24 '10 at 12:58
  • May it's better use Form.Close instead of Application.Exit also (if possible) http://stackoverflow.com/questions/1057151/application-exit – Andrew Florko May 24 '10 at 12:59
  • @Andrey You're right, the logic of the statement doesn't make clear sense...but I can't see the rest of his/her code to know what the right logic would be. all I can see is that he's obviously using the wrong operator to do his comparison. – Justin Niessner May 24 '10 at 13:01
0
 if (userName.Length > 4 || userName.ToLower() != "student")
 {
     Application.Exit();
 }

Try this.

Dave Zych
  • 21,581
  • 7
  • 51
  • 66
Ta01
  • 31,040
  • 13
  • 70
  • 99
  • The `ToLower()` may be negligable depending on how the UI accepts input. Also it may be a case sensitive username... – James May 24 '10 at 13:00
  • still safer imo, sometimes you wonder why I condition isn't hitting and its simply a matter of case. – Ta01 May 24 '10 at 13:04
  • @Random: Only if case is not important. – James May 24 '10 at 13:14
  • 1
    Don't perform case-insensitive string comparison using ToLower() -- it fails the Turkey test. Use String.Equals with a case-insensitive StringComparison argument instead! http://www.moserware.com/2008/02/does-your-code-pass-turkey-test.html http://msdn.microsoft.com/en-us/library/t4411bks.aspx – dtb May 24 '10 at 13:16
  • Good points, still wondering why people down voted this answer twice, atleast give me a reason – Ta01 May 24 '10 at 13:45
0

From looking at your requirement your check for the length of chars is negligable or at least you haven't mentioned the reason why you want to check length of the char. From the example you have provided I would simply check if (userName != "student") I don't see the need for the extra check this is something that can be forced in the UI.

James
  • 80,725
  • 18
  • 167
  • 237
0

If I am not wrong, you are trying to enter into the if loop when either of the condition is true. i.e., enter into the if loop

  1. when Length is greater than .
  2. when userName should not be equal to "student"

condition like username = "ABC" is not "student", your condition still right and will enter. It will execute still when userName is equal = "student";

Here You should use AND operator than just OR operator.

if (userName.Length > 4 & userName != "student")
  {
   Application.Exit();                
  }

You can also achieve the same result with && operator. & opertor is same as && operator. when X = false and Y is true; Y will not be evaluated at all. Since X is already false. this is method is also call short-circuit evaluation.

Harsha
  • 1,861
  • 7
  • 28
  • 56