- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
A means of setting a breakpoint on one of the return conditions using just simple static breakpoints..... Removing it would not "Break" operations, but would eliminate a potentially useful debugging/diagnostic point.
Admin
You can't set a breakpoint on a line of non-code, so your guess is impossible.
Admin
one line of code might have been scrubbed: // TODO
Admin
BatchProcessor.ProcessBatch() sure sounds like it triggers a side effect...
Admin
First iteration will be adding an "Intentionally left blank" comment in each branch.
Admin
Looks like someone forgot the throw new NotImplementedException() line in both branches.
Admin
Maybe they got a compiler warning that "return value from ProcessBatch() is unused", so they "fixed" it by doing this.
Admin
That explains the
if, it doesn't explain theelse.Admin
That depends on the language. This is possible in C#, but I'm not sure about other languages.
Admin
Template inserted by IDE.
Admin
I pretty much have to do that because of a SonarQube rule we have turned on. If you have an "if/elseif" construct, SQ says it's a code smell if you don't have a final "else".
Admin
At the risk of not being snarky enough, my guess is that either: a) there used to be code in there that was rendered obsolete so it got removed, but the programmer didn't want to risk removing the if/else; or 2) the programmer was expecting a meaningful distinction, which then never materialized.
Either way, @TrayKnots may be right, there may be a side effect of that .ProcessBatch() method. Of course, it could instead be simply assigned to a dummy variable (hopefully with comments to that effect) so that it happens, but avoiding the useless If.
Admin
Another possibility: The programmer wrote it but didn't know what to do in the failure case. His request for clarification went unanswered.
Admin
<quote>everyone is just hesitant to touch something that works and they don't understand.</quote>
Absolutely. If you change the code, then it needs to be edited, compiled and re-deployed. Something in that process might break; and thus YOU caused the system to fail. Whereas, if you make no change at all, it can't be your fault if something breaks.
If it works, don't fix it.
Admin
Easy: the code in the if and else clauses is written in Whitespace
Admin
I like the lisp concept of
(when COND BODY ...). It allows to make it clear that there is no else. In such a language, an if without else is indeed a code smell.More generally though?
Admin
Technically you can't set a break point on a line without code in C# (or any .net language) as well, but you can set them at the end of a scope (which is pretty much what { } mean in C like languages), because scope ends are usually also injected with break statements (in .net they are nop's, natively it depends on the platform). And in release compilation often code is eliminated (as are empty branches already by the IL compiler, long before NGEN finally gets rid of them in case of using [Conditional] for example), so you can't really properly debug non-debug code for that simple reason. Same also is true for C/C++, there's just no IL compiler, because you already compile natively. And most other languages behave in a similar way, most can have break points at the end of a scope.
Admin
I was working with clients in the past where some style guides demanded an else statement for every if statement. It doesn't make sense obviously, bloats the code unreasonably, but well, usually there were else branches with comments, just to shut up StyleCop.
Admin
Can do it in Javascript and Java too.
Rather useful at times I must say.
Admin
My bet is that he refactored a bunch of ugly code out, piece by piece, and then just had to leave the empty conditional block behind as a symbolic monument to that work. Probably planned to pull it out soon afterwards and forgot.
Admin
Is an empty if or else block anot a code smell then.
Writing code like thIs to satisfy code quality tools certainly is smelly. If you dont understand why a rule exists you should either learn why or remove the rule.
Admin
So? Remove the if. Obviously nothing changes if you do.
You might want to check inside the function if it does some error handling though.
Admin
Actually it does. If this is actually a.method meant to be called incrementally, the bool could indicate an idle state which would be useful for further handling following request of pause processing for a while ;-)
Admin
@MaxiTB - Thanks... especially with debug versions of some "C" compilers I have seen for mainframe applications (not totally out of the question for a financial application)
Admin
Many years ago I was working on C code using Microsoft's C compiler for a DOS based program. I spent a long time trying to figure out a bug in the program. The issue was that if I put a debugging statement in the code, the code would work. I took out the line and the code would fail. I finally looked at the machine language that was being generated and the code was wrong. So, I called Microsoft (this was before the days of email) and tried to get support on it. I even went so far as to create a tiny app, (source code and makefile) that would trigger the error. I released my code with a simple zz=x+1; which was s do nothing line and everything worked fine. Got a letter back from Microsoft stating they couldn't reproduce the error I was claiming. Not long after I got an update disk for the compiler and poof, bug was fixed.