|
I think, because the THEN branch is short, the second is more natural.
EDIT: I turns into mess when the same condition evaluated many times in the "sentence". Natural languages and regexp have simple backreference to solve the problem.
|
|
|
|
|
I prefer
if(inVal==NULL) inVal = internalDefault;
Why waste a line of screen space. It's easy enough to read.
Though I actually prefer the declaration assign the argument w/ the valid default value as in
return_type whatEver(arg_type inVal=internalDefault);
I simply do not understand the logic of assigning a value to an argument only to immediately discard and replace it. I would be happy to learn of same.
Incidentally I learned from a previous post or article I don't recall which to write e.g.
void foobar(int arg)
{
if(arg != valid_value) return;
}
instead of
void foobar(int arg)
{
if(arg == valid_value)
{
}
}
so as to minimize the number of indented blocks even though in previous times I preferred minimizing the number of return statements. I find minimizing the number of indented blocks to be easier to understand. - Cheerios
|
|
|
|
|
Comments below assume C#, capabilities / syntax in other languages may vary.
My preference is, if internalDefault is not a compile-time constant, to overload the function e.g.
void whatEver() => whatEver(internalDefault);
void whatEver(sometype inVal) { }or (assuming internalDefault is a compile-time constant)
void whatEver(sometype inVal = internalDefault) { }Better than null would be to use default(type) e.g.
void whatEver(sometype inVal = default(sometype) { }But in all of theses you should still check that inVal is not null.
Alternatively, you can use the null coalescing operator e.g.
void whatEver(sometype inVal = null)
{
inval = inval ?? internalDefault;
}
One day, perhaps, there will be a null coalescing assignment operator so you can do
inVal ??= internalDefault; - this is a feature that I have wanted in JavaScript since JS1.1 (c 1998) to change having code like
myvar = myvar || somedefault; into
myvar ||= somedefault;
|
|
|
|
|
My God in heaven. The first thing you absolutely must fix is the spacing:
inVal = (inVal == NULL) ? internalDefault : inVal; That's better. Now, what were you babbling on about?
Software Zen: delete this;
|
|
|
|
|
Gary Wheeler wrote: That's better. Now, what were you babbling on about?
Shoes and Ships and Sealing Wax. Cabbages and Kings.
| Ravings en masse^ |
|---|
| "The difference between genius and stupidity is that genius has its limits." - Albert Einstein | | "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010 |
|
|
|
|
|
I would completely disregard both causes, check what my developers commonly understand best, and pick that one.
As a rule:
- Never write code for an unqualified or unasked efficiency goal.
- Always write code that is cost-effective to maintain.
I've seen many many many average developers write code in a specific way because it's supposedly more efficient.
In almost all of those cases, they completely skipped measuring performance and examining the software requirements, and are picking an obtuse implementation because it makes them feel good about their code.
To be a excellent developer, you need to write excellent code for humans, and randomly add optimized of code for the compiler.
The compiler doesn't care at all, while the humans do. Write code for the latter.
|
|
|
|
|
KateAshman wrote: check what my developers commonly understand best, and pick that one.
I've never considered coding to be a majority operation.
I do what I do because I think that's how it should be done. If I learn something better I'll fix it.
KateAshman wrote: I've seen many many many average developers write code in a specific way because it's supposedly more efficient.
Seems to contradict your earlier (first) statement.
Don't join the herd in a stampede of "me too!" - if everyone does everything because that's how everyone else does it then nothing will change.
| Ravings en masse^ |
|---|
| "The difference between genius and stupidity is that genius has its limits." - Albert Einstein | | "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010 |
|
|
|
|
|
Coding is a team sport, even when you're alone: you need to collaborate with you from the past and you from the future.
Writing code that's first and foremost efficient, while not actively improving existing code for specific performance goals, is a bad idea. I'm implying that without evidence and without specific performance goals, you should always write code that's concise and easy to understand instead. Easy to understand, however, is a moving target, which depends entirely on your team and your coding language.
I've extensively researched the topic of code quality for 20+ years. YMMV
|
|
|
|
|
I would always use the first set of code (marked by "this ?") for two reasons...
1)
I haven't bothered to learn all the new ways of coding for the simple reason that they most often make the code look arcane and difficult to read. This would result in boosting maintenance costs through difficulties with less experienced developers trying to understand what has been written.
2)
Efficiency is highly overrated in computer systems. Do you really believe that a Human Being will be able to tell the difference in speeds between the two types of code? Of course not. So why bother with it?
The reason developers bother with such concoctions is they are under the impression that what they are doing either looks "cool", they actually believe that using such arcane coding constructs will make their applications perform better, or both. As to the latter, no such coding construct will make one's code perform better except in the tiny recesses of computer memory where no one will ever notice.
Steve Naidamast
Sr. Software Engineer
Black Falcon Software, Inc.
blackfalconsoftware@outlook.com
|
|
|
|
|
Efficiency is not highly overrated, depending upon the type of coding you do.
Even early on I had some molecular modeling code that, when I changed from function (actually subroutine) return values to global values ran about 95% faster. Overnight batch runs became real-time in terms of excitation and planning the next iteration target. OK - long ago with FORTRAN and room-size computers. And time sharing. Some other optimizations were made for I/O timing, as well.
Back to now: doing it right, which means efficient coding, is even necessary for Web-Based applications: whether it's load time for a page (one second or fifteen?) or, far more importantly, optimizing SQL: something which has reared it's head, recently, as the amount of data in tables, especially joined tables, has grown. A stored procedure could even fail do to exceeding the maximum execution time set up (again, 400 users means limits).
So - if all your coding has to handle is small numbers of iterations and much time awaiting human responses it makes no perceivable difference - but good habits pay one back when they don't have to what always seems to be the inevitable future of a good application.
| Ravings en masse^ |
|---|
| "The difference between genius and stupidity is that genius has its limits." - Albert Einstein | | "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010 |
|
|
|
|
|
I don't think it matters much as the compilers are pretty efficient at this.
In C#, I usually use the "null coalescing assignment operator (??=)"; so "inVal ??= internalVal;".
Here are some functions to test the various ways to assign a default value:
void FixNullArg_SimpleIf(string arg = null) {
if (arg == null)
arg = "Fix null via simple if";
Console.WriteLine(arg);
}
void FixNullArg_TernaryOp(string arg = null)
{
arg = (arg == null) ? "Fix null via ternary operator" : arg;
Console.WriteLine(arg);
}
void FixNullArg_NullCoalescingOp(string arg = null)
{
arg = arg ?? "Fix null via ?? operator";
Console.WriteLine(arg);
}
void FixNullArg_NullCoalescingAssignmentOp(string arg = null)
{
arg ??= "Fix null via ??= operator";
Console.WriteLine(arg);
}
void FixNullArg_IsNullOrWhiteSpace(string arg = null)
{
arg = string.IsNullOrWhiteSpace(arg) ? "Fix null via IsNullOrWhiteSpace" : arg;
Console.WriteLine(arg);
}And here is the decompiled Intermediate Language (IL):
FixNullArg_SimpleIf:
IL_0000: nop
IL_0001: ldarg.1
IL_0002: ldnull
IL_0003: ceq
IL_0005: stloc.0
IL_0006: ldloc.0
IL_0007: brfalse.s IL_0010
IL_0009: ldstr "Fix null via simple if"
IL_000E: starg.s 01
IL_0010: ldarg.1
IL_0011: call System.Console.WriteLine
IL_0016: nop
IL_0017: ret
FixNullArg_TernaryOp:
IL_0000: nop
IL_0001: ldarg.1
IL_0002: brfalse.s IL_0007
IL_0004: ldarg.1
IL_0005: br.s IL_000C
IL_0007: ldstr "Fix null via ternary operator"
IL_000C: starg.s 01
IL_000E: ldarg.1
IL_000F: call System.Console.WriteLine
IL_0014: nop
IL_0015: ret
FixNullArg_NullCoalescingOp:
IL_0000: nop
IL_0001: ldarg.1
IL_0002: dup
IL_0003: brtrue.s IL_000B
IL_0005: pop
IL_0006: ldstr "Fix null via ?? operator"
IL_000B: starg.s 01
IL_000D: ldarg.1
IL_000E: call System.Console.WriteLine
IL_0013: nop
IL_0014: ret
FixNullArg_NullCoalescingAssignmentOp:
IL_0000: nop
IL_0001: ldarg.1
IL_0002: brtrue.s IL_000B
IL_0004: ldstr "Fix null via ??= operator"
IL_0009: starg.s 01
IL_000B: ldarg.1
IL_000C: call System.Console.WriteLine
IL_0011: nop
IL_0012: ret
FixNullArg_IsNullOrWhiteSpace:
IL_0000: nop
IL_0001: ldarg.1
IL_0002: call System.String.IsNullOrWhiteSpace
IL_0007: brtrue.s IL_000C
IL_0009: ldarg.1
IL_000A: br.s IL_0011
IL_000C: ldstr "Fix null via IsNullOrWhiteSpace"
IL_0011: starg.s 01
IL_0013: ldarg.1
IL_0014: call System.Console.WriteLine
IL_0019: nop
IL_001A: ret I'm sticking with the ??= operator for simple null tests and ternary when more involved checks are needed (e.g. see if a string is null, empty or just whitespace).
modified 5-May-21 13:19pm.
|
|
|
|
|
You should not write off the compiler so soon. A C++ compiler probably generates the same code for either statement. So you can have cosmetic and efficient.
|
|
|
|
|
In ASM/C/C++, efficiency and performance always outweigh readability. If a Padawan cannot read it, he is not ready to become a Jedi.
|
|
|
|
|
This looks like a script function not in C/C++. As for this example in VC++
void whatEver(int inVal) { <br />
<br />
if(inVal==0)<br />
inVal = internalDefault;<br />
<br />
inVal = (inVal==0)?internalDefault:inVal;<br />
}
See disassembly code in Debug build as simply:
; 16 :
; 17 : if(inVal==0)<br />
<br />
cmp DWORD PTR _inVal$[ebp], 0<br />
jne SHORT $LN2@whatEver<br />
<br />
; 18 : inVal = internalDefault;<br />
<br />
mov DWORD PTR _inVal$[ebp], 123 ; 0000007bH<br />
$LN2@whatEver:
While the other just
; 21 : inVal = (inVal==0)?internalDefault:inVal;<br />
<br />
cmp DWORD PTR _inVal$[ebp], 0<br />
jne SHORT $LN4@whatEver<br />
mov DWORD PTR tv66[ebp], 123 ; 0000007bH<br />
jmp SHORT $LN5@whatEver<br />
$LN4@whatEver:<br />
mov eax, DWORD PTR _inVal$[ebp]<br />
mov DWORD PTR tv66[ebp], eax<br />
$LN5@whatEver:<br />
mov ecx, DWORD PTR tv66[ebp]<br />
mov DWORD PTR _inVal$[ebp], ecx
But you can't see both in Release build because both optimized in compilation.
|
|
|
|
|
You are overthinking this.
First - it is script-like. The C++ was just a posting choice for formatting - apparently a poor choice on my part as many others also though I meant in the C++ context.
Second - this was really a question on preferences. Aside from your disassembly, the clear difference is one would (as written) always MOVe a value and the other only conditionally - both doing the same conditional test. The question is, for most applications, which would you rather see - in your own code and someone else's your stuck looking at.
The script style of the function, very PHP-like, was to illustrate the function declaration and content all in one small location. In a PHP script roughly like this it acts in a similar manner to a C++ function overload (one with an arg, one without).
On the other hand, I appreciate your thoughts and dis-assembly. I've not disassembled code ( once upon a time I had a x86 commenting disassembler) in a very many years.
| Ravings en masse^ |
|---|
| "The difference between genius and stupidity is that genius has its limits." - Albert Einstein | | "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010 |
|
|
|
|
|
Yea, I see your point here on preferences. When saw the subject like More Efficient, it often made me think about actual implementation details. But fine, thanks for your good explanation
|
|
|
|
|
Keep in mind that modern optimizers are *very* good. I wouldn’t assume that your two examples are actually going to result in different compiled code. Definitely lean toward readability.
|
|
|
|
|
It was always about readability although I did mention that it saved a step. sometimes, in what I think is the more readable version.
Spread across all languages where it's a possibilitya, however, includes scripting languages and compiled versions.
| Ravings en masse^ |
|---|
| "The difference between genius and stupidity is that genius has its limits." - Albert Einstein | | "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010 |
|
|
|
|
|
if(inVal==NULL)
{
inVal = internalDefault;
}
Ternary operators have their place, but aren't mean to replace "if" blocks. Also using that since the input-validation exceptions follow the same pattern, makes code nicely readable. And yes, always as blocks. Typing two chars extra isn't gonna kill anyone.
Bastard Programmer from Hell 
"If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
|
|
|
|
|
I would never want to be in a situation where I had to justify something that was redundant but looked better.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it.
― Confucian Analects: Rules of Confucius about his food
|
|
|
|
|
It's all in the subject my child.
|
|
|
|
|
Help me, Obi-Ken - you're my only hope!
"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony
"Common sense is so rare these days, it should be classified as a super power" - Random T-shirt
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
1977 i saw it in the movie theaters it was awesome, and i have been a fan ever since
|
|
|
|
|
I had the opposite reaction. I realized that I was not a Sci-Fi fan.
"the debugger doesn't tell me anything because this code compiles just fine" - random QA comment
"Facebook is where you tell lies to your friends. Twitter is where you tell the truth to strangers." - chriselst
"I don't drink any more... then again, I don't drink any less." - Mike Mullikins uncle
|
|
|
|
|
Why does Skywalker always use chopsticks – just use the fork Luke.
"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony
"Common sense is so rare these days, it should be classified as a super power" - Random T-shirt
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
