8

I've got the following and was wondering if the initial test is overkill:

static void Main(string[] args) {
    if (args.Length == 0 || args == null) {           
        //do X
    }
    else { 
        //do Y 
    }
}

in other words what I'm asking is are there possibilities of args.Length being zero, or args being null....or would just one of these conditions sufficed?

whytheq
  • 34,466
  • 65
  • 172
  • 267
  • 6
    Your logic is the wrong way round. What happens if `args` is `null`? Checking `args.Length` will throw a `NullReferenceExcetpion` – Greg B Mar 19 '12 at 08:23
  • overkill? not really. unless you absolutely need to provide a response when arguments are required to execute the program, then do it. Greg B is right, check if null then check length. – Martin Ongtangco Mar 19 '12 at 08:24
  • As a general rule, you should check for null before anything else, because if `args` is indeed null, your first condition will throw a `NullReferenceException`. – Ioannis Karadimas Mar 19 '12 at 08:24

5 Answers5

15

Well, Main is defined to never ever ever ever be called with a null parameter. If it does somehow receive a null parameter, then your environment is so broken that all bets are off no matter what you do, so there's really nothing to be gained by checking for null.

On the other hand, if you do check for null, then readers and maintainers of the code will have to understand why. Why did the original programmer put in such a useless check? Did he know something we don't? We can't just remove it, because he might have uncovered some weird corner case bug!

In other words, you're adding complexity to your program, and tripping up future readers of the code. Don't do that. That future user might be you. Make your future self happy, and write code that makes sense.

However, in situations where such a null check does make sense, it must be the leftmost condition.

In a test like this: args.Length == 0 || args == null, args.Length is evaluated first, and if that fails, args is compared to null. In other words, if args is null, your code will throw an exception. It should be args == null || args.Length == 0

jalf
  • 243,077
  • 51
  • 345
  • 550
  • That's a truly terrible answer. All good developers know not to rely on the behaviour of external code when it calls their public functions. Defensive programming always trumps Hope programming (i.e hoping the parameter is not null). It's really bad practise to get into the habit of assuming parameter values. Check for null - there's no harm in it, you can't really claim it's a performance hit in this instance and it shows that you are thinking of all possibilities. – adelphus Mar 19 '12 at 10:24
  • 4
    I never said anything about performance. But please tell me how it is bad to rely on your programming language and its class library *to do what it says*? If you can't rely on that, how can you write **any** kind of code? If you can't trust that, then every single line you write, **including your null check** is suspect and unreliable. If you want to talk about terrible practices, then this kind of pointless obfuscation is it. Saying "what if *everything* in the whole world is corrupt and unreliable? What will we do then? I know, checking for `arg == null` will fix it". No, it won't. – jalf Mar 19 '12 at 10:34
  • 3
    If your world is broken, then you can't fix it, and you might as well not bother to try. You can call it "hope" programming if it makes you feel better, but it's the only thing we can do. We have to *hope* that the compiler generates the code we asked for. We have to hope that the `List` class actually stores a list of T's, we have to *hope* that `Main` will be called on startup. We have to *hope* that accessing a `null` object throws a `NullReferenceException`. Defensive programming can't protect you against a broken language implementation. – jalf Mar 19 '12 at 10:35
  • 1
    Who said the only caller is the .NET Framework? This has nothing to do with the language implementation - it's all about good programming practises. This question could easily be asked about any other language. Defensive programming is all about making sure the program behaviour is known and well understood in the case of errors. If you don't do it and assume everything will be OK, you're waiting for a world of pain when it all comes crashing down. – adelphus Mar 19 '12 at 10:47
  • 3
    If I was riding a bicyle why would I need to wear a motor bike helmet - _in case one day in the future_ the bicycle sprouts an engine. Surely it's enough to stick with wearing a bicycle hat, like jalf says if somebody gets the bicycle in the future with a motor bike helmet they'll wonder whats going on! (maybe not the best analogy I've thought up) – whytheq Mar 19 '12 at 12:11
  • From simple question born an interesting discussion... :) – Tigran Mar 19 '12 at 12:24
  • 1
    @whytheq I would argue that you're not wearing a helmet at all :-) Everyone has told you that no cars are ever going to pull out in front of you so you think it's OK. Problem is, I've seen loads of cars swipe loads of bikes (metaphorically speaking). Personally, I don't want to die and I don't want anyone who uses my bike to die either. – adelphus Mar 19 '12 at 14:50
  • 2
    @adelphus: Then you may want to mandate that any rider wear a 10-foot-thick protective suit and parachute, and carry a nuclear missile in case of falling asteroids. Face it, you can't -- and shouldn't! -- protect against every weird thing that might ever happen. Consider (to get back onto the topic :)) that if something calls `Main(null)`, something is fundamentally broken. That something would be *the caller*, and that's where the fix should go. `if (args == null)` is a band-aid on a broken leg. – cHao Mar 19 '12 at 16:40
  • it's all gone a bit too surreal - this is a serious site for serious analysis...better keep my cheap analogies to myself next time – whytheq Mar 19 '12 at 17:20
  • +1, for the 11 comment long discussion-argument-debate thing. – ApprenticeHacker Mar 20 '12 at 08:13
  • 4
    @adelphus: "This question could easily be asked about any other language." Seriously? So defensive programming is about protecting against the case *where your code suddenly turns out to be a different language*? I can just imagine it. "Oh shit, the C++ code I've been writing for the last 8 months just turned out to be Prolog! Damn, if only I'd protected against `main` being called with invalid parameters". This question is about the `Main` method in *one* specific language. In this language, Main is called once, by the runtime, at application launch, unless the programmer is insane – jalf Mar 20 '12 at 09:09
  • 2
    and if the programmer is insane, a null check isn't going to help us. – jalf Mar 20 '12 at 09:11
  • @whytheq: "If I was riding a bicyle why would I need to wear a motor bike helmet - in case one day in the future the bicycle sprouts an engine" -- that is a **great** analogy. Cracked me up. +1 :) – jalf Mar 20 '12 at 09:12
  • I concede I've lost this battle against the Nay's. @jalf you clearly don't understand what defensive programming is or why it's used. I do admit that *in this particular instance* there is a high probability that the parameter will not be null. However, I view SO as a learning forum as well as a QA and hopefully I've at least given other readers food for thought. – adelphus Mar 20 '12 at 10:03
  • 2
    @adelphus: I do know what defensive programming is. But the OP asked a *specific* question about checking for null *in a very specific scenario*. I absolutely agree with you, in many *other* cases, it makes sense to check for null. But we're not talking about those cases. We're talking about a C# program where the Main method checks if its argument is null. SO is a learning forum, and one of the important lessons every programmer should learn is that "it depends". Blindly checking **everything** against null is just as stupid as checking *nothing*. – jalf Mar 20 '12 at 10:20
  • if(argument == discussion) { GiveUp();} – whytheq Mar 20 '12 at 13:30
  • 1
    one of the best answers on SO – vidstige Oct 12 '12 at 08:36
  • @jalf just to clarify, none of `args`s' **values** could be null either right? (assuming the runtime calls it of course, not some crazy programmer) – Ohad Schneider Jul 22 '17 at 10:44
10

According to this, you only need to check :

if (args.Length == 0)
{
    // Do X
}

Though checking for null doesn't do any harm, there is no real need.

ApprenticeHacker
  • 21,351
  • 27
  • 103
  • 153
  • 2
    +1 for Occam's Razor and providing a link. The other answers are correct, of course, for general cases, but this question was very specific. – Mr Lister Mar 19 '12 at 08:30
  • @InternediateHacker - nice one; I thought I remembered there being an msdn entry just couldn't find it! – whytheq Mar 19 '12 at 09:53
3

It's never bad idea to make additional control, if this is not into the high performance, very frequent used functions. So I would say, not, it's not overkill.

And another thing: first check for null, after for Length

Tigran
  • 61,654
  • 8
  • 86
  • 123
  • 5
    Yes, it is absolutely a bad idea to add code which serves no purpose. It clutters up the code, make it harder to read and to understand, and, quite simply, gives you more code to maintain. `Main` does not get called with a null parameter, so don't add complexity to your code trying to test for *what if it does* – jalf Mar 19 '12 at 08:25
  • Well, your `Main` method should not normally be used more than once per program execution ;-) – Péter Török Mar 19 '12 at 08:26
  • @jalf: I never used till now program where Main method is used more then once, don't know if you did. The think is that if **suppose** that one day, after some service pack (update) it *can* become null your program will not fail. Just add additional control and live happy. Simple. – Tigran Mar 19 '12 at 08:32
  • 1
    @Tigran: Service packs don't change the language spec to break existing programs. And **if** they do, then it takes more than a null check to fix your program. If such a service pack was issued, it would break much more than this. But imagine you didn't *write* this code, but *read* it. Imagine you had to maintain it. My reaction when reading it would be "what the heck? Why is this null check necessary? Does the original author know something I don't?" – jalf Mar 19 '12 at 08:35
  • 1
    And then I'd spend the rest of the day trying to uncover whatever bug he was obviously protecting against, and research cases where `args` could be null. Time wasted because *someone* introduced unnecessary complexity into his code. If the .NET framework doesn't follow its own spec, we're screwed no matter what we do. Writing your code on the assumption that .NET lies to you is futile – jalf Mar 19 '12 at 08:36
  • 1
    @Tigran: One day, after some service pack, `Console.WriteLine` may cause unicorns to appear and format your hard drive. You can't and shouldn't defend against every case that "might" happen in the future -- consider that if such breaking changes were made, you'd have a lot more to worry about than `args` being null. – cHao Mar 19 '12 at 08:36
  • @cHao: could you be more precise? It's not really clear if it's the service pack or the unicorns that'd format your harddrive. I just want to be sure. ;) – jalf Mar 19 '12 at 08:44
  • @jalf: Not sure. I see a correlation between unicorns appearing and hard drives mysteriously getting formatted. Can't say for sure it's the unicorns that do it, though. – cHao Mar 19 '12 at 08:47
  • @jalf: understand your point, but you can not convince me that presence of one null reference check in the code will make it less readable, create a confusion to you and force to loose a time on *study* (???) the code you looking at. – Tigran Mar 19 '12 at 09:24
  • 2
    Perhaps I can't. But if not, then you haven't had to maintain a lot of code. Simplicity is important. Understanding *why* the code looks the way it does is *extremely* important. If you can't give a good reason for why your code looks the way it wants, I don't want to maintain it. – jalf Mar 19 '12 at 09:28
  • @jalf: agree on this point +1. Unfortunately maintaining the old (legacy) code is a part of my job, so share your thoughts. – Tigran Mar 19 '12 at 09:31
  • 1
    Sorry guys, but all those who believe that you should not protect against all cases are simply bad developers. I've encountered soooo many instances where a developer has responded to a program crash with: "Well the parameter should never have that value" that it beggars belief. Relying on other people's word that parameter values to public functions will only contains certain values is a fatal mistake. – adelphus Mar 19 '12 at 10:30
  • @adelphus: agree. Don't like that "I know what I'm doing" and after recieve calls in midnight from some client for the problem that the program is crashed and he lost all his data.Becasue *exactly* that scenario unfortunately happens many time in my life. Prefer to have a check, log and user notification on operation failure. In this way client doesn't loose anything (even if not able to finish it's job, but it's better) . – Tigran Mar 19 '12 at 11:44
  • 1
    @adelphus: There's a difference between "shouldn't have" and "won't have", or even "can't have". In this case, `args` will never be null (with one exception...if someone specifically calls it within the app and passes `null`). This isn't a library function; it's part of the spec for the CLR itself. It's not just that it "shouldn't" happen; it *will not happen* unless you call `Main` (incorrectly!) from within your app...or something has gone so wrong that you can't trust the runtime. And if you can't trust the runtime to even start your app properly, how can you trust anything from that point? – cHao Mar 19 '12 at 13:45
  • 1
    I love it when people say " will never happen....except when ". I love it so much that I often hug the developer when happens and someone has to clear up the remains. – adelphus Mar 19 '12 at 14:54
  • 1
    @adelphus: There is precisely *one* case where this will happen. And if that case occurs, whatever *called* `Main(null)` is **broken**. Period. When the rules of the game are set and you refuse to play by them, all bets are off. In this case, the rules are set by the CLR itself. When they're broken, the code should break. *Gleefully.* And whoever adds `if (args == null) { /* oh noes */ }` to their code, rather than fixing the caller (which is in the same freaking app), needs to be strung up for their incompetence. – cHao Mar 19 '12 at 16:23
  • 1
    @adelphus: you're missing the point. It is not that " will never happen", but that "if happens, we're screwed no matter what, and what you're proposing would not fix the program if happened". You know what *I* love? I love it when programmers say " would never normally happen, and if it does, there is nothing we can do to recover, so you know what I'll do? I'll write a lot of code which tries to recover *anyway*" – jalf Mar 20 '12 at 10:33
  • 1
    I love it so much that I often hug the developer when happens and someone has to not just fix the error, but *also* fight his way through these layers and layers of obfuscated *useless* code which serves no purpose but to hide the error that occurred, and which *still* needs to be fixed – jalf Mar 20 '12 at 10:33
  • @Tigran: which scenario happens often in your life? That the CLR breaks in a way so that defensive coding practices is able to save the day and avoid midnight calls from the client? I find that unlikely – jalf Mar 20 '12 at 10:35
  • 1
    @jalf: no : ) Hopefully not CLR. Happens often that developer *suppose*: "this will nevere happen", and in 90% it **will** happen. I'm talking about **not** this concrete case the question was asked for. Our discussion went far away from actual topic of the question . – Tigran Mar 20 '12 at 10:37
  • @jalf I *really* hope that when a paying customer contacts you, your response is: My software deliberately crashes because that's what I want it to do when something isn't right. – adelphus Mar 20 '12 at 10:38
  • 1
    @Tigran: well, **some** of us are trying to answer the OP's question. The OP did not ask about defensive programming in general. He didn't ask about that case last week where your client got pissed at you for not checking if some parameter was null. He asked if it was a good idea to check C#'s `Main` method's argument against null. – jalf Mar 20 '12 at 10:40
  • @jalf: there are different *real software worlds* out there. Not all have a good test units (if has), good developers or good managers. Smeone work directly with clients (which has its benefits and troubles for software developer). That "diffencive" way of programming is usual used by those ones that are closely work with the client(s). Others, who works in the office, in calm and separated from outside world with organized customer service, can and will chooose other ways. I think this is just a business. – Tigran Mar 20 '12 at 10:41
  • 1
    @Tigran Absolutely. I would argue that's it more like 95%. It's even better when the developer says: "I'm 100%/110%/1000% certain that this will never happen". And then of course, it does. It appears that experience counts in this case. – adelphus Mar 20 '12 at 10:41
  • @adelphus: I *really* hope you're not charging money for software which tries to camouflage errors and just continue in an unreliable state where it might do far more damage than if it'd just crashed. See, I can do that too. Perhaps we could stick to the topic at hand for five minutes? – jalf Mar 20 '12 at 10:42
  • 1
    @adelphus: and yet I just asked if **the problem we are talking about guarding against** ever happened, and he said no. How about you? Has it happened for you? And if it did, can you seriously, honestly, say that this null check would have saved the day? Let me try again. I am not saying "I am certain this parameter will never be null". I am saying "I am certain that **if** this parameter is ever null, then my environment is so screwed up that nothing I can do will matter." – jalf Mar 20 '12 at 10:43
  • 1
    Personally, I vote to close this question with the reason that it may cause unnecessary emotional stress :-) – adelphus Mar 20 '12 at 10:44
  • 1
    If Main is called with a null argument, I have no reason to believe that my code will even be executed correctly. And so, even if I did insert that null check, I can't even be sure that it would be executed, or that it wouldn't do something else, like, say, formatting the customers harddrive". You might as well write code to verify that your compiler emitted correct code or that the CPU isn't defective. Those are clear errors that we'd love to catch, but we can't, because if they occur, none of our code can be relied upon. Just like we can't rely on our code if the CLR suddenly breaks – jalf Mar 20 '12 at 10:45
  • The problem is @jalf, developers take your reasonings and apply them in other places. All I'm trying to do is to indicate that good programming practises involve good parameter checking - which includes checking for null. A developer could easily view your comments and say "Well in my API, that integer parameter will never be more than 3 because it makes no sense in my function. Therefore, I won't bother checking for that case". It's bad practise and it really does cause problems. Last comment on the topic (I promise). – adelphus Mar 20 '12 at 10:55
  • @adelphus: A 'developer' that reads "the CLR will never call `Main(null)`" and sees "i don't have to check whether someone passed me a 4"...eh. Maybe he should be checking for null, for the sole purpose of making it clearer to the rest of the world that he don't know what the hell he's doing. Those are two different cases. No one's said that you shouldn't check the stuff that other user code passes to you. But you can trust the system to do its job -- or rather, if you can't, then you can't really trust anything running in the system either (even your own code!). – cHao Mar 20 '12 at 18:00
  • I understood that could make a question on topic and get plenty points on it :) Really amazing how the simple question leads to *related* long battle where are no winners... – Tigran Mar 20 '12 at 18:57
  • Amazingly sad, maybe. This really shouldn't have turned into what it did. – cHao Mar 21 '12 at 06:44
  • @cHao: why sad?? This id Q&A and also discussion forum. It's good to have someone who can offer different solutions. The solutions, may be, I'm not convince in or didn't even think about. – Tigran Mar 21 '12 at 07:47
  • @Tigran: Sad because it was a straightforward yes/no question. There's not much room for "different solutions". That, and the answers i was arguing with are so clearly *wrong*. :P It's blind paranoia and cargo cultism. – cHao Mar 21 '12 at 12:15
3

If there's no input data, then args.Length is equal to 0, but not null. If there's any input data, then agrs.Length is equal to count of input arguments. In conclusion, args can't be null, but length can be zero.

PS check for null first always

Rinat
  • 598
  • 1
  • 8
  • 18
0
if (args == null)
{
    Console.WriteLine("args is null"); // Check for null array
}
else
{
    if (args.Length == 0)
    {
        //do X
    }
    else 
    { 
        //do Y 
    }
}
Dor Cohen
  • 16,769
  • 23
  • 93
  • 161
  • You can simplify this by using `if (args == null) { ... } else if (args.Length == 0) { ... } else { ... }` and save a layer of curly braces. FWIW, in most cases the branches for `null` and empty array are probably the same anyway. – Joey Mar 19 '12 at 08:28
  • @Joey about the brackets I think that this is more readable for others, and about the second comment you're wrong, it case of args is null, Checking args.Length will throw a NullReferenceException! – Dor Cohen Mar 19 '12 at 08:45
  • 4
    That's why you check *first* for `null` and *then* for `Length`: `if (args == null || args.Length == 0) ...` – Joey Mar 19 '12 at 09:13