|
Then you'ld get a null reference...
Just like I said.
But seeing how this is a private method, it's ok to write it like that most of the time imo.
It would be a different story if the method was public.
|
|
|
|
|
No...
It's programmer porno..!! lol
|
|
|
|
|
I disagree with the sentiment that badly readable code can or should be augmented through comments. Comments are a means to increase maintainability, and if there is any reasonably easy way to rewrite the code into something better readable or even self-explaining, then that is a much better way to add maintainability than writing a comment.
Whether or not there is a comment, the line of code above might require several minutes of contemplation to grasp it's meaning for anyone unfamiliar with this code. A well written alternate code segment however will be understandable within seconds, even without comments.
|
|
|
|
|
(((user.Roles & userRole) != 0) ? inRoles : outRoles)
...this is the part that needs some 'splaining. Maybe something as simple as:
var rolesList = (user.Roles & userRole) != 0) ? inRoles : outRoles;
rolesList.Add(roleName);
|
|
|
|
|
This is what I was thinking as well, which is why I looked at all the responses. The only I would change, in .NET 4, is to use the HasFlags method of enum:
var rolesList = user.Roles.HasFlag(userRole) ? inRoles : outRoles;
rolesList.Add(roleName);
This also means no brackets are necessary.
|
|
|
|
|
Good catch ...I was actually unaware of the HasFlag method ...nice!
|
|
|
|
|
I like it. But why make it a function. Since it is just a single line, just leave it in place and you don't waste time with a function call. Only if called in several places does it need to be a function.
But, I have friends that tell people that I can (and am willing to) write a whole program on a single line. 
SS => Qualified in Submarines
"We sleep soundly in our beds because rough men stand ready in the night to visit violence on those who would do us harm". Winston Churchill
"Real programmers can write FORTRAN in any language". Unknown
|
|
|
|
|
First, the advantage of terse code is that you can more of the code on the screen at a time. If that advantage outweighs others, such as possibly the next guy getting confused, maybe it's worth it. Maybe you're writing code on a 40 column x 20 row terminal like an Atari 800. This likely may not be the case.
The next issue is optimization. Does this code optimize better than the equivalent:
if ((user.Roles & userRole) != 0)
inRoles.Add(roleName);
else
outRoles.Add(roleName);
I bet the rewritten version isn't any less optimized.
Third, which one is easier to single-step through? Some debuggers only let you step through lines, not statements.
And finally, if you're doing this so that people will go, "wow, you can do that? that works? wow", then you're likely to be breaking the Principle of least astonishment.
ken@kasajian.com / www.kasajian.com
|
|
|
|
|
Firstly, no, Visual Studio on a 22" monitor, so I don't have that excuse.
Secondly, if the two do not optimize to the same code, then someone as Microsoft should be up against the wall.
Thirdly, I think the VS compiler will allow single step only on the second version.
Finally, no, I did it because it seemed reasonable at the time, after I had removed a pile of code (which is why it has a dumb name - it didn't for long and got deleted completely soon afterwards) When I realized what I had left myself, the initial response was "Yeuch"! So I stuck it here.
It realized some interesting responses - from my point of view it is pretty nasty, and not something I would want to leave in production code. Interesting that some people seem to think it is fine, if it is commented!
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
So why do you think some people think it's okay?
I've heard programmers tell me that swapping the value of x and y without using a temporary intermediate is somehow better, coding ending up looking something like this:
*x^=*y^=*x^=*y
eeek
ken@kasajian.com / www.kasajian.com
|
|
|
|
|
Kenneth Kasajian wrote: So why do you think some people think it's okay?
http://www.codeproject.com/Messages/4008174/Re-Is-this-a-coding-horror.aspx[^]
Kenneth Kasajian wrote: *x^=*y^=*x^=*y
In embedded assembler, when you are after every last clock cycle in a limited uProcessor, then swap by XOR can be a real time saver - since it only uses the ALU, there is less external memory access, which can save a lot of time. I don't like to work that close to the wind though - and if I do, it get commented to hell and back. In C++ or C#? Don't go there! 
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
That's brilliant. Although in the spirit of this thread, it should surely be commented:
*x^=*y^=*x^=*y;
|
|
|
|
|
Your code is not a horror in any ways!
Instead, its a clever code and must be upvoted as I have given 5up for that!
|
|
|
|
|
It looks perfectly simple to me. I'd be very disappointed if a programmer couldn't understand it!
|
|
|
|
|
hmm... The only thing I'd argue with is doing all that then adding the adding the "Add" method to the end. Being used to the ternary operator, I could read it fairly quickly, but to make it more human readable, perhaps an assignment a variable should have been assigned to the chosen list, then the second line should have implemented the Add method.
In ANSI C in college, they encouraged this type of stacking behavior. In fact, they demanded it, but it was more of a performance issue than simply being "clever."
|
|
|
|
|
Came across this piece of code in a source library 
string sParam1 = _param1;
string sParam2 = _param2;
int iOut1 = -1, iOut2 = -1;
if (Int32.TryParse(sParam1, out iOut1) && Int32.TryParse(sParam2, out iOut2))
{
Int32.TryParse(sParam1, out iOut1);
Int32.TryParse(sParam2, out iOut2);
}
Me and my colleague ended up  !! believe me or not this piece is written recently by the one known as the lead!!
|
|
|
|
|
Nothing unusual to see here. He's just checking twice to make sure it's really parsable.
It's like when I'm just about to exit my flat and close the door behind me, I check if I have the keys with me. It doesn't really matter that I already checked that before embarking on my way to the door. I just need to make sure.
You know? Like really, really sure. Sometimes checking only once is just not enough.
"With sufficient thrust, pigs fly just fine."
Ross Callon, The Twelve Networking Truths, RFC1925
|
|
|
|
|
Why do you think it's called "double-checking"? 
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
posting about Crystal Reports here is like discussing gay marriage on a catholic church’s website.[Nishant Sivakumar]
|
|
|
|
|
Exactly what I was trying to say, I guess!
"With sufficient thrust, pigs fly just fine."
Ross Callon, The Twelve Networking Truths, RFC1925
|
|
|
|
|
The person who wrote this code is probably suffering from OCD[^]. 
|
|
|
|
|
That poor guy got very confused with terrible threading issues!
|
|
|
|
|
Bad enough its VB, they are even using a select for two cases when an Or would have sufficed.
Select Case stringvalue.ToLower
Case "a", "b"
rslt = Format(CDbl(AmountB), "#,0.00")
GoTo skpy
If CheckPPSO Or CheckPRS Then
If DownPayment >= Downpayment_Cutoff Then
If CountryAbbr = "CA" Then
rslt = Format(GetPrice(CodeLetter, "XYZ"), "#,0.00")
Else
rslt = Format(GetPrice(CodeLetter, "X"), "#,0.00")
End If
End If
skpy:
End If
End Select
This was in the middle of a larger switch with 246 case statements. Good thing it's Friday, I've stopped processing data after coming to this.
Why are people who do this even allowed near computers  
No comment
|
|
|
|
|
Ooh, nice, the "comment out code using a goto" pattern! 
|
|
|
|
|
At least the compiler should have shown you where this one was (warning: unreachable code detected). But yes that is pretty bad!
|
|
|
|
|
I don't code in VB, so I could be well off the mark here, but isn't that GoTo also going to take you to a point where your next statment is "End If", without having actually executed the corresponding "If"? Couldn't that have "interesting" effects?
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
