|
That only applies to objects. For instance...
var a = 'howdy';
var b = a;
a = 'partner';
alert(b);
var x = {message: 'howdy'};
var y = x;
x.message = 'partner';
alert(y.message);
And even if that was the coder's intent, it would still be way more readable to do something like this...
var a = 'blah';
var b = new String(a);
a = 'yo';
alert(b);
Jeremy Falcon
|
|
|
|
|
Well, substring omits the character at the index specified by the second parameter, so the code you listed will return attributeVal with its final character removed.
Maybe that's not what this code should be doing in this case, but what you showed isn't obviously incorrect. It's exactly the code you'd want to use to remove the last character of a string, although
attributeVal.slice(0,-1) would work in this case too.
OTOH,
var fieldName = attributeVal.substring(0, attributeVal.length);
would be pointless, as it would just return attributeVal
|
|
|
|
|
Sanjay K. Gupta wrote: It is javascript. You don't say!
The language is JavaScript. that of Mordor, which I will not utter here
This is Javascript. If you put big wheels and a racing stripe on a golf cart, it's still a f***ing golf cart.
"I don't know, extraterrestrial?"
"You mean like from space?"
"No, from Canada."
If software development were a circus, we would all be the clowns.
|
|
|
|
|
That made my day.
Jeremy Falcon
|
|
|
|
|
There are increasing numbers of questions being posted with code similar to:
try {
callAPIMethodThatReturnsStatus(some parameters)
}
catch (SomeException e) {
take exceptional action
}
display ('The method completed successfully');
And yet they never check the returned status of the method call. Someone is teaching students/newbies that if it does not throw an exception then it must have worked. This seems most common with database update commands.
Get your money out of the bank quickly.
|
|
|
|
|
Maybe the return type of callAPIMethodThatReturnsStatus was poop and the programmer didn't want to deal with that s#!t.
Kitty at my foot and I waAAAant to touch it...
|
|
|
|
|
Not to mention the headless use of try-catch - I wonder who will pay the bill at the end...
Skipper: We'll fix it.
Alex: Fix it? How you gonna fix this?
Skipper: Grit, spit and a whole lotta duct tape.
|
|
|
|
|
Kornfeld Eliyahu Peter wrote: I wonder who will pay the bill at the end Oh, that will be us poor customers.
|
|
|
|
|
Well, I saw such type of code also at the then biggest vendor of hospital information systems...
But they were one step better than your example: they stored the return value of the call to third party in a variable, which they never checked. That's an enormous step forward already, isn't it?
|
|
|
|
|
Reading this thread reminds me of a short story that I read long ago. I can no longer remember whether I was in high school or college; the only reason that has any relevance is that I have forgotten the story, but not its title, "Insert Flap A and Throw It Away."
I have learned, and am periodically reminded of its importance, that if a function returns a value, even if it is poop, you would do well to check it. After all, if the author thought it was important enough to return, who am I to brush him off by ignoring it?
Plenty of authors have written plenty of functions that return void. In all versions of BASIC, these are called Subs, short for Subroutines. Their author is effectively saying, "I have nothing to report; just trust me."
A couple of weeks ago, I ran across a method that returns void that I wish didn't. The method in question, Console.WriteLine() would be much more useful if it returned a character count, along the lines of what you can get from printf() , but probably aren't. Almost every example I see that uses printf() doesn't bother with the return code. This is so prevalent that I hadn't given the matter any consideration until I started working with its buffered cousin, sprintf() . Along the way, I discovered that both have a return value of type int, which returns the number of characters written. While the newer "secure" print functions in the latest Visual C runtime library return minus one to indicate failure, even the old ones can be persuaded to print nothing and return zero.
David A. Gray
Delivering Solutions for the Ages, One Problem at a Time
Interpreting the Fundamental Principle of Tabular Reporting
|
|
|
|
|
Cause
The C# code includes an empty string, written as “”.
Rule Description
A violation of this rule occurs when the code contains an empty string. For example:
string s = "";
This will cause the compiler to embed an empty string into the compiled code. Rather than including a hard-coded empty string, use the static string.Empty property:
string s = string.Empty;
How to Fix Violations
To fix a violation of this rule, replace the hard-coded empty string with string.Empty.
So..
string s = string.Empty;
..is more readable than..
string s = "";
Since when are two words more "readable" than the two symbols that half the world knows and uses? The word "hard coded" is equally out of place - it feels like it means to warn that the constant "might" change and that it therefore must be wrong to hard-code it.
Yes, very likely scenario.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
What about
string s = "";
vs
string s = "";
(yeah, yeah - I know that syntax colouring should make this sligtly obvious, and that the IDE will then underline it all wiggly-like, and then the compiler will barf. But still...)
I think the problem here is they are saying it's "readability" when in fact it's more about efficiency and not creating a new object each time.
cheers
Chris Maunder
|
|
|
|
|
Chris Maunder wrote: What about That is the only advantage, which is a non-problem if you have a decent font.
Chris Maunder wrote: I think the problem here is they are saying it's "readability" when in fact it's more about efficiency and not creating a new object each time. If the motivation is incorrect or nonsense, the rule is ignored.
Didn't know that it was a micro-optimisation, as I didn't expect a "new object" for a string-literal.
--edit
Still thanks for the explanation
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
According to SO: c# - String.Empty versus "" - Stack Overflow[^] the "new object each time" bit might have changed in later versions.
And this String.Empty Versus ""[^] would imply that it has indeed been changed.
I prefer
string s = ""; anyway - it's just more readable to my mind.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
It doesn't create a new object. String.Empty and "" point to the same spot in memory.
#SupportHeForShe
Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson
You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
|
|
|
|
|
Making SA1122 nonsense
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
why not use:
var s = string.empty?
|
|
|
|
|
First, there is no need for "var"; it is preferred to use the string-datatype. Second it would also be preferred to use a literal (or a constant pointing to the literal), not a class-member - that's what this entire thread was about.
"var" would be useful if it was a linq-query, but it is not meant as a replacement for those cases where you know what type to expect.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
There is huge advantage of using String.Empty over "" - it has nothing to do with readability...
Skipper: We'll fix it.
Alex: Fix it? How you gonna fix this?
Skipper: Grit, spit and a whole lotta duct tape.
|
|
|
|
|
Kornfeld Eliyahu Peter wrote: it has nothing to do with readability... According to StyleCop it is about readability, which it obviously isn't.
Kornfeld Eliyahu Peter wrote: There is huge advantage of using String.Empty over "" Yet you do not explain what the advantage might be, where the disadvantages are obvious.
..if there's a yuuge advantage, then I'd like to know. Please, point it out
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Isn't that obvious?! Using String.Empty will remove that idiotic SA1122!
Skipper: We'll fix it.
Alex: Fix it? How you gonna fix this?
Skipper: Grit, spit and a whole lotta duct tape.
|
|
|
|
|
No, string.Empty is rubbish; you need System.String.Empty .
As to the issue with "" ; how does the (uninformed) reader know there aren't any non-printing characters in there?
|
|
|
|
|
PIEBALDconsult wrote: No, string.Empty is rubbish; you need System.String.Empty . Did you mean global::System.String.Empty ?
PIEBALDconsult wrote: As to the issue with "" ; how does the (uninformed) reader know there aren't any non-printing characters in there? The uninformed reader should not come near code and keep to his VB6 code; said reader would also not notice if I'd created another local String-class with a string-member called "Empty" that returns "GET OUT OF MY CODEBASE".
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
There is no one size fits all and VS code analysis rule set is no exception. If you don't like this particular rule just disable it. Either globally or in place optionally with justification.
I actually use string.Empty as it is more readable for me. Shorter code =/= more readable (try only single letter class members...).
There are other rules that I find annoying however. Like "GetSomething()" - replace with getter, or "method don't use instance members - use static". Both are cool, except in unit tests. But SuppressMessage attribute handles it nicely there.
Overall quality of our code significantly improved when we stared threat CA warnings as errors and reject all pull requests with SupressMessage without Justification. But that is with custom rules as ever evolving company standard
--
"My software never has bugs. It just develops random features."
|
|
|
|
|
Deflinek wrote: If you don't like this particular rule just disable it. It is there for a reason, which may or may not be valid, but I do want to know the reasoning behind it. My liking is irrelevant, I want to weigh the implications of both scenario's.
Deflinek wrote: I actually use string.Empty as it is more readable for me. Shorter code =/= more readable (try only single letter class members...). No, it is not more readable for you. You might prefer it, but it is not more readable. It takes more symbols to convey the same information, meaning your brain will have to do more validation.
v equalz Onehundredandeighty plus sixtynine is not more readable than
v = 180 + 69; That's not even a matter of preference. I do admit that shorter code is not more readable by definition; it does not help if things are obfuscated, but that's hardly the case here, is it? We are simply re-using a long-accepted version of "empty string" (known as such in various languages), instead of calling a (static?) member on an class (which is local to the language, without adding any benefits that are known to me).
Deflinek wrote: There are other rules that I find annoying however. It is not that I find it annoying, I'm challenging its validity.
Deflinek wrote: Overall quality of our code significantly improved when we stared threat CA warnings as errors No arguing that, every warning is one too many.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|