rofl! Man, that smells so much like a “Damn, it’s 18:00 and this has to demo tomorrow morning.” Don’t think, just do.

A rare instance where saying something loud enough really does make it true.

I’m not saying it’s pretty code, but it does do something. It’ll correct the capitalization of the Role variable on the customer object.

Obviously that shouldn’t be the Customer objects responsibility though

Sorry Steve but I can´t believe that a good programmer would do that even in a hurry…

@Eric : It’s not about whose responsibility! This ain’t ayende’s post .

If you see closely, “Role” has never been assigned, so it will have a Null value.

Should I take it you guys would like to see more of these pearls?

Who said anything about it being a “good” programmer? Well, at least a good, “sober” programmer.

Here’s a doosy I had to sift out of a production system recently. Ironically, AFAIK you cannot do this in C#…

Public Interface IRiskBusinessLayer
…
Function LoadPreviousVersionsOfRiskReviews(ByVal riskID As Integer) As dstResolvedRisk.ReviewDataTable
Function LoadFinalisedRiskReviews(ByVal riskID As Integer) As dstResolvedRisk.ReviewDataTable
…
End Interface

Public Class RiskBusinessLayer Implements Risk.IRiskBusinessLayer
…
Public Function LoadPreviousVersionsOfRiskReviews(ByVal riskID As Integer) As dstResolvedRisk.ReviewDataTable Implements IRiskBusinessLayer.LoadFinalisedRiskReviews
…
End Function

Public Function LoadFinalisedRiskReviews(ByVal riskID As Integer) As dstResolvedRisk.ReviewDataTable Implements IRiskBusinessLayer.LoadPreviousVersionsOfRiskReviews
…
End Function
End Class

Let s/he who has not N 1′d cast the first stone!

1) Considering the fact that we compare Role and role for equality and if they are (uppercase) equal we set them (again) to be equal.
The only (possible) sense in this would be if we would have two different roles with different casing (which I doubt is the case)

2) Considering the fact that Role is null at the beginning of the loop, this if statement would never evaluate true and Role would always stayed empty.

3) The only way how this could work is that someone do

var customer = new Customer();
customer.Role=”Admin”;
customer.Save();

in which case the complete looping is useless (unless #2)

Yeap, it is a beauty

My first thought was that Roles.GetAllRoles() could somehow be accessing the entire array/file/database of customers, which means there’s a chance that the first value of role to come up would be the one already assigned to the Customer. So, for example, you may have customers with roles “BOSS”, “CEO”, “peon”, “BOSS” and “Peon”. Suppose we’re looking at the customer with role “Peon”; iterating through the list in the order given would change that to “peon”, which may or may not be a bad thing depending on how the boss, Boss and BOSS happen to feel about their staff… But of course that would be silly… right?

Should that crash if Roles.GetAllRoles() actually returns something? When you call ToUpper() on the null role string, it would get a NullReferenceException.

@Andrew Hall
Haha… Just noticed someone else caught that.

Yes, it is correcting the casing of the Customer’s Role string. I don’t see anything wrong with the concept, if that’s the requirement. Needs some refactoring–its own function at least–but it’s not stupid in itself.