If statement not running - iOS

Question

I have a iOS app with a few simple and logical if-statements. However they wont run and I can't understnad why. Here is my simple code:

-(void)viewDidLoad {

    [super viewDidLoad];

    // Run setup code.

    int day = [self currentDay];

    if ((day == (1 || 3 || 7) && (day != (2 || 4 || 5 || 6))) {

        // Run the calendar setup code for
        // Sunday/Tuesday OR saturday.
        [self runSetupVX_4];
    }

    else if ((day == (2 || 4 || 5 || 6)) && (day != (1 || 3 || 7))) {

        // Run setup code for Monday
        // wednesday, thursday and friday.
        [self runSetupVX_2];
    }
}

-(int)currentDay {

    NSDateComponents *component = [[NSCalendar currentCalendar] components:NSCalendarUnitWeekday fromDate:[NSDate date]];
    return [component weekday];
}

What am I doing wrong here?

what do you mean that i will not run? Does your code has any errors? Or do you mean that it doesn't get into either of the statements? — hoya21, Jul 15 '15 at 13:34
I've never seen this `day == (1 || 3 || 7)` syntax before in an `if statement`. Where did you get this from? — Popeye, Jul 15 '15 at 13:38

score 6 · Accepted Answer · answered Jul 15 '15 at 13:28

6

This compiles, but it does something completely different from what you are trying to achieve:

day == (1 || 3 || 7) && (day != (2 || 4 || 5 || 6)

The statement ORs 1, 3, and 7, and then ORs 2, 4, 5, and 6, before performing comparisons.

A proper way to do it is to compare day to 1, 3, and 7 separately. If any of the comparisons is successful, it's also guaranteed that the day is not 2, 4, 5, or 6:

if (day == 1 || day == 3 || day == 7)
    ...
if (day == 2 || day == 4 || day == 5 || day == 6)
    ...

You could also rewrite this with a switch:

switch(day) {
    case 1:
    case 3:
    case 7:
        // Run the calendar setup code for
        // Sunday/Tuesday OR saturday.
        [self runSetupVX_4];
        break;
    case 2:
    case 4:
    case 5:
    case 6:
        // Run setup code for Monday
        // wednesday, thursday and friday.
        [self runSetupVX_2];
        break;
}

answered Jul 15 '15 at 13:28

Sergey Kalinichenko

714,442
84
1,110
1,523

But with your if function, you have lots of copies of ```day ==``` this is why I was trying the one in my question. Anyway I like the ```switch``` method that you written. I will use that as it doesn't have an code copies. Thanks. – Johnysmith Jul 15 '15 at 13:50
@Johnysmith Objective-C does not offer a built-in syntax for "x is a member of collection" operation, so you need to repeat `day == ...` part when comparing to multiple constants. When the list is short, it does not matter, because the code remains clear to a human reader. When the list gets longer, `switch` offers a nicely readable alternative. – Sergey Kalinichenko Jul 15 '15 at 14:09
While I'm not going to vote down this answer, I have to disagree with it. Look at @Skrew answer. It does what any programmer should be doing: it keeps the code simple and clean. I think your answer is overkill..... – Jul 15 '15 at 14:14
@Dan Can you point out a major difference between the answer you like and my answer? I mean, apart from my answer explaining what's wrong with OP's code, offering two alternatives instead of one, and explaining why the `!=` part is redundant? – Sergey Kalinichenko Jul 15 '15 at 14:20
@Dan this is totally the right answer. Just because an answer is short and simple doesn't make it the acceptable answer. This answer has explanations and alternatives the other doesn't – Popeye Jul 15 '15 at 14:25
1

@Dan this answer is providing an alternative so they could either do `if (day == 1 || day == 3 || day == 7) {} else {}` or they could do `if (day == 2 || day == 4 || day == 5 || day == 6) {} else {}` or they could go the way of the switch case statement. And one persons **opinion** doesn't mean that they are right. I've never heard anyone ever say that `switch case` is bad practice. This answer still remains the best answer here. – Popeye Jul 15 '15 at 14:36
@Dan Knowing when to *omit* `break`s in a `switch` is a very basic skill, so I am inclined to think that you have misunderstood rmaddy's statement about it being "bad practice". I suspect that rmaddy was talking about a situation when a `case` has some code and no `break` at the end, which isn't necessarily bad practice, but has somewhat higher chance of being a bug. – Sergey Kalinichenko Jul 15 '15 at 14:41
2

@Dan You have definitely misunderstood what `rmaddy`'s comments are saying. He never says that it is bad practice for one and he is saying that the switch case is bad for that specific situation and prone to bugs not that switch case are always prone to bugs. You have completely misread what he has said. And I'd probably be careful what you are saying as I wouldn't say `"Says the person with buggy code :) Are you prepared to put money where your mouth is?"` is being friendly and is actually quite rude. – Popeye Jul 15 '15 at 14:48
@Popeye Did you not read his comment? It clearly said: "The code in this answer is safe. Typically it's a bug when the break is missing but in a situation like this, it is what you actually want. As a convention, I always add a comment in place of the missing break clarifying that is really shouldn't be there.". He is CLEARLY saying that it IS a bug :) – Jul 15 '15 at 14:51
1

@Dan that in no way says it is a bug. You have clearly misunderstood. All he is saying is when you have a switch case like this where a group of cases all do the same thing so you miss out the break so it jumps to the next case statement he advises it is best to put a comment so someone else coming a looking at the code knows that is what it is meant to do. That in no way means it's a bug. Note the `The code in this answer is safe` and his comments are specifically for that situation and has nothing to do with this question. – Popeye Jul 15 '15 at 14:54
1

@Dan You misunderstood what rmaddy calls "a situation like this". The linked answer does with a switch what can be done with a loop, but more importantly, `break`s are missing in **non-empty** cases. – Sergey Kalinichenko Jul 15 '15 at 14:58
3

@dasblinkenlight Ah.... so I was the idiot all along.... ah.... Well then in that case I agree to an unconditional surrender. I am defeated! – Jul 15 '15 at 15:08

score 4 · Answer 2 · edited Jul 15 '15 at 14:37

4

You're overcomplicating it a bit.

 if(day==1 || day==2 || day==3){
     [self runSetupVX_4];  
 }else{
     [self runSetupVX_2];
 }

Should do the trick

edited Jul 15 '15 at 14:37

Popeye

11,839
9
58
91

answered Jul 15 '15 at 13:28

Skrew

1,768
1
16
17

3

This should be the accepted answer. Its the most simple and practical solution out of all the answers. – Jul 15 '15 at 13:52
@Dan I agree. I like the simplicity of this answer. The other answers are very similar to the OPs code. – Supertecnoboff Jul 15 '15 at 13:56
@Dan I would disagree with you, this shouldn't be the accepted answer as the accepted answer from `dasblinkenlight` actually answers the question and provides a very good in-depth answer and also provides alternatives. Just because an answer is simple doesn't mean it is necessarily the correct one. – Popeye Jul 15 '15 at 14:22
@Popeye At least this answer doesn't have bugs.... – Jul 15 '15 at 14:32
1

@Dan in what way does the other one have bugs? If you are going to come out with statements like that you need to be able to back them up. – Popeye Jul 15 '15 at 14:36
@Popeye Well lets put it simply, if you were looking to hire a programmer, would you hire one which writes overly complicated code... OR... would you like to hire someone who keeps code simple and clean? Keeping code simple and to the point is vital. What if you have a massive program, you could end up making mistakes and loosing track of what the code does. Keeping it simple, makes it easier for you and other programmer to understand your code :) – Jul 15 '15 at 14:54
@Dan If I was hiring someone to do my code I would hire the person with the best/most experience who knew what they were doing. So again that would be `dasblinkenlight` as they know exactly what they are doing and that can be seen in their code and the explanations that come with that code, and the explanations of where someone has gone wrong. Whereas this answer has none of that, this answer doesn't show experience as a developer at all (No disrespect to `Skrew` there). – Popeye Jul 15 '15 at 15:00
@Popeye That makes logical sense. I can't argue with that. – Jul 15 '15 at 15:06

score 0 · Answer 3 · answered Jul 15 '15 at 13:41

If the first part of each if statement is true, then the second part will be false every time. So you don't have to check the second part.

You can modify your code like this:

-(void)viewDidLoad {

[super viewDidLoad];

// Run setup code.

int day = [self currentDay];

if (day ==1 || day==3 || day==7) {

    // Run the calendar setup code for
    // Sunday/Tuesday OR saturday.
    [self runSetupVX_4];

}
else{

        // Run setup code for Monday
        // wednesday, thursday and friday.
        [self runSetupVX_2];


}}

-(int)currentDay {

    NSDateComponents *component = [[NSCalendar currentCalendar]          components:NSCalendarUnitWeekday fromDate:[NSDate date]];
    return [component weekday];
}

score 0 · Answer 4 · edited Jul 15 '15 at 14:17

0

Hope this works for you:

if (day == 1 || day == 2 || day == 3) {

    // Run the calendar setup code for
    // Sunday/Tuesday OR Saturday.
    [self runSetupVX_4];
}

else if (day == 2 || day == 4 || day == 5 || day == 6) {

    // Run setup code for Monday
    // Wednesday, Thursday and Friday.
    [self runSetupVX_2];
}

edited Jul 15 '15 at 14:17

AHiggins

7,029
6
36
54

answered Jul 15 '15 at 13:43

Manoj Chandel

508
5
12

score -3 · Answer 5 · answered Jul 15 '15 at 13:24

-3

You can put break point at if statement and then check that if statement is get called or not.

answered Jul 15 '15 at 13:24

Pradumna Patil

2,180
3
17
46

I think using ```breakpoints``` would be a much better way to debug whether or not the if statements are being executed. – Jul 15 '15 at 14:02
This is a comment not an answer – Popeye Jul 15 '15 at 14:30

score -3 · Answer 6 · edited Jul 15 '15 at 14:23

-(void)viewDidLoad {

  [super viewDidLoad];

  // Run setup code.

  int day = [self currentDay];
  //if ((day == (1 || 3 || 7) && (day != (2 || 4 || 5 || 6))) {   
  //here u will get 0 or 1 from this  (1 || 3 || 7) && (day != (2 || 4 || 5 || 6))) 
  //And u r comparing with 0 or 1 with day  so please check below code how to compare 

  if (((day == 1 || 3 || 7) && (day != 2 || 4 || 5 || 6)) {

    // Run the calendar setup code for
    // Sunday/Tuesday OR saturday.
    [self runSetupVX_4];
  }

  else if ((day == 2 || 4 || 5 || 6)&& (day != 1 || 3 || 7)) {

    // Run setup code for Monday
    // wednesday, thursday and friday.
    [self runSetupVX_2];
  }
}

-(int)currentDay {

   NSDateComponents *component = [[NSCalendar currentCalendar] components:NSCalendarUnitWeekday fromDate:[NSDate date]];
   return [component weekday];
} //you have problems with comparing variable please check it

This answer is no better then the question code. Please read comments on other answers. This `day == 1 || 3 || 7` is not correct syntax for objective-c for doing what they want. — Popeye, Jul 15 '15 at 14:24

If statement not running - iOS

6 Answers6