|
Ive taken huge advantage of this recently when using PerformanceCounters. You see I create performance counters if possible, but it's entirely expected that some developer machines might not yet have my PerformanceCounters and I didnt want my code to not work on their machines.
Hence this Extension class
public static class PerformanceCounterExtensions
{
public static void SafeIncrement(this PerformanceCounter counter)
{
if (counter != null)
counter.Increment();
}
public static void SafeDecrement(this PerformanceCounter counter)
{
if (counter != null)
counter.Decrement();
}
public static void SafeIncrementBy(this PerformanceCounter counter, long value)
{
if (counter != null)
counter.IncrementBy(value);
}
public static void SafeReset(this PerformanceCounter counter)
{
if (counter != null)
counter.RawValue = 0;
}
}
|
|
|
|
|
If only you could have extension... parentheses... or whatever you'd call it... seriously, Microsoft, WHY did you have to choose null to represent an event that has no handlers?! Now instead of
MySpecialPropertyChanged();
I have to do
if (MySpecialPropertyChanged != null)
MySpecialPropertyChanged();
or have the program crash if nothing's listening to the event!
|
|
|
|
|
You can, in C++. And you don't get an exception/runtime error until your program tries to touch data members - much deeper in the call stack than the actual call point. Which makes debugging a very funny thing ...
|
|
|
|
|
The compiler does not read an extension method as a method belonging to the instance object, instead it expands your code to something like:
Console.WriteLine(ExtensionMethods.IsNull(o));
Therefore, you do not get an exception.
|
|
|
|
|
I am not a C# person, but in C++, you can call a static method through a pointer that is NULL:
{
CBlah* pBlah = NULL;
pBlah->SomeStaticMethod(); CBlah::SomeStaticMethod(); }
Calling the method requires the right access level (ie. private/protected/public) and a value may or may not be returned by the API (ie. 'void', 'int' etc.). This is not valid in java, and I don't know if it is valid in C#.
--
Harvey
|
|
|
|
|
The fellow who wrote this (shall remain nameless ) several years ago was enamored of the ternary operator (hmm... Lispy). It seemed to him rather clever at the time; that was, until he encountered it again during maintenance. The variable name he chose turned out to be somewhat prophetic in a phonetic sort of way.
byte[] arySend = FetchCommand(cmd.CHANGE_THE_ACTIVE_DISPLAY);
int iScreen = this.cboDisplayName.SelectedIndex;
iScreen = iScreen < 8 ? iScreen + 1 :
(7 < iScreen && iScreen < 10 ? iScreen + 4 :
(10 == iScreen ? iScreen + 5 :
(11 == iScreen ? iScreen + 10 :
(11 < iScreen && iScreen < 16 ? iScreen + 12 :
(15 < iScreen && iScreen < 20 ? iScreen + 24 :
(19 < iScreen && iScreen < 22 ? iScreen + 45 :
(22 == iScreen ? iScreen + 53 :
(23 == iScreen ? iScreen + 57 :
(23 < iScreen && iScreen < 26 ? iScreen + 65 :
(25 < iScreen && iScreen < 28 ? iScreen + 72 :
(27 < iScreen && iScreen < 30 ? iScreen + 152 :
(30 == iScreen ? iScreen + 160 :
(30 < iScreen && iScreen < 33 ? iScreen + 169 :
(33 == iScreen ? iScreen + 170 :
(34 == iScreen ? iScreen + 171 :
(35 == iScreen ? iScreen + 175 :
(35 < iScreen && iScreen < 41 ? iScreen + 176 :
(41 == iScreen ? iScreen + 203 :
(42 == iScreen ? iScreen + 208 :
iScreen = iScreen + 212 )))))))))))))))))));
arySend[2] = Convert.ToByte(iScreen);
arySend[3] = 0x01;
|
|
|
|
|
Nice
"Any sort of work in VB6 is bound to provide several WTF moments." - Christian Graus
|
|
|
|
|
|
David MacDermot wrote: enamored of the ternary operator
I love the ternary operator too, but this...this...is just too much.
Please poke my eyes out now!
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
This is actually a fairly elegant construct if done correctly – a value-returning if-else stack, switch/case, or in this case quantiser into ranges. He's obfuscated it with the parens and indentation, and the unnecessary lower bound checks. If written as
iScreen = iScreen < 8 ? iScreen + 1 :
iScreen < 10 ? iScreen + 4 :
iScreen < 11 ? iScreen + 5 :
iScreen < 12 ? iScreen + 10 :
iScreen < 16 ? iScreen + 12 :
iScreen < 20 ? iScreen + 24 :
iScreen < 22 ? iScreen + 45 :
22 == iScreen ? iScreen + 53 :
23 == iScreen ? iScreen + 57 :
iScreen < 26 ? iScreen + 65 :
iScreen < 28 ? iScreen + 72 :
iScreen < 30 ? iScreen + 152 :
30 == iScreen ? iScreen + 160 :
iScreen < 33 ? iScreen + 169 :
33 == iScreen ? iScreen + 170 :
34 == iScreen ? iScreen + 171 :
35 == iScreen ? iScreen + 175 :
iScreen < 41 ? iScreen + 176 :
41 == iScreen ? iScreen + 203 :
42 == iScreen ? iScreen + 208 :
iScreen + 212;
... or, in fact, since the iScreen+ is in all of them, like
iScreen = iScreen +
iScreen < 8 ? 1 :
iScreen < 10 ? 4 :
iScreen < 11 ? 5 :
iScreen < 12 ? 10 :
iScreen < 16 ? 12 :
iScreen < 20 ? 24 :
iScreen < 22 ? 45 :
22== iScreen ? 53 :
23== iScreen ? 57 :
iScreen < 26 ? 65 :
iScreen < 28 ? 72 :
iScreen < 30 ? 152 :
30 == iScreen? 160 :
iScreen < 33 ? 169 :
33 == iScreen? 170 :
34 == iScreen? 171 :
35 == iScreen? 175 :
iScreen < 41 ? 176 :
41 == iScreen? 203 :
42 == iScreen? 208 :
212;
... it's not too bad. It's still not the right way to code this, which would be a map of upper bound to delta
OrderedMap<int, int> screenDeltas = { 7=>1, 9=>4, 10=>5, ..., 42=>208, 43=>212 };
function getScreenDelta(iScreen){
foreach(MapEntry<int, int> delta in screenDeltas)
if(iScreen <= delta.Key) return delta.Value;
}
modified 2-Aug-12 5:16am.
|
|
|
|
|
It can be done 'better' (I think I have done up to 4 with that style before). I would likely land up using a switch for all the == cases, and have the < cases in the default case (using the ternary operator style you laid out).
Otherwise, I don't really like the ordered map. Another way to do it would be to:
int[] screenDeltas = new[] { 1, 1, 1, 1, 1, 1, 1, 1, 1, 4, 4, };
iScreen += iScreen < 0 ? 1 :
iScreen > 42 ? 212 :
screenDeltas[iScreen];
(Clearly code would be used to generate that array initializer)
Because array lookups are O(1) and I love my premature optimization.
He who asks a question is a fool for five minutes. He who does not ask a question remains a fool forever. [Chineese Proverb]
Jonathan C Dickinson (C# Software Engineer)
|
|
|
|
|
I don't see the problem, just look at the comments...
____________________________________________________________
Be brave little warrior, be VERY brave
|
|
|
|
|
The code quality is reminiscent of the legacy code I used to maintain that was written in the 70's and early 80's, before Ed Yourdon et. al. started talking about structured programming, which is itself a very old paradigm. I count a cyclomatic complexity of 51 (academia says 10 is "maintainable", really 18 or so if you stretch it) and it's full of "magic numbers" that have no obvious meaning other than they appear to be "screen numbers". It is kind of depressing to see that kind of code being written in a modern language, 30 years after the first efforts were made to educate people out of these kinds of practices.
|
|
|
|
|
I believe Yourden wrote about structured programming in the late 1960's, and it was well known to professionals then. Your legacy code writers had no excuse. (Like they ever do...)
There must be a selection-bias thing at work here wherein well-written legacy code doesn't need to be maintained, because it's well-written, and the legacy code that needs to be maintained, needs maintenance because it is bad code.
The other possibility is intolerable; that there are some kids in middle-school today who will be cursing our beautiful code in 20 years. That would mean that even great coders aren't great every day. Just because you're Mozart doesn't mean people will remember ALL your music. Lots of it was produced under deadline pressure, and was only ok.
|
|
|
|
|
Wouldn't it be easier to bind the values of each item in the checkbox to the value this thing would produce and then just take the SelectedValue?
|
|
|
|
|
Yes, or at least bind the items to an object, of which this magic number was a property.
|
|
|
|
|
BobJanova wrote: Yes, or at least bind the items to an object, of which this magic number was a property.
That is exactly what I did when I had the pleasure of encountering this code while creating the next generation of the application.
I employed a java enum style class in C#.
public class Display
{
public static readonly Display OpeningSplashdisplay =
new Display(1, "Opening Splash display");
public static readonly Display StartUpDateConfig =
new Display(2, "start up date config");
public static IEnumerable<Display> Values
{
get
{
object temp = null;
FieldInfo[] fields = typeof(Display).GetFields();
foreach (FieldInfo field in fields)
{
temp = field.GetValue(null);
if (temp is Display)
yield return (Display)temp;
}
}
}
private readonly byte value;
private readonly string name;
Display(byte value, string name)
{
this.value = value;
this.name = name;
}
public byte ToByte() { return value; }
public override string ToString() { return name; }
public static Display ParseByte(byte value)
{
foreach (Display d in Display.Values)
{
if (d.ToByte() == value)
return d;
}
return Display.OpeningSplashdisplay;
}
}
Here I load the combobox from the data structure.
cmbDisplay.Items.AddRange(new List<Display>(Display.Values).ToArray());
And this is how I handle the selection
private void cmbDisplay_SelectedIndexChanged(object sender, EventArgs e)
{
ComboBox c = (ComboBox)sender;
Display selected = (Display)c.SelectedItem;
_RawData[2] = selected.ToByte();
}
|
|
|
|
|
First, send my congratulations to your fellow, because he have a superior understanding of the use of the ternary operator, being smart (and brave) enough to nest that number of levels (the max i can nest is 4 whitout being lost in the logic). Second, slap him in the face, because this is a nightmare to maintain and a shame to see in any program, given that even a humble switch-case would have been better than this mess.
|
|
|
|
|
Bill Gates is a very rich man today... and do you want to know why? The answer is one word: versions.
Dave Barry
Read more at BrainyQuote[ ^]
|
|
|
|
|
The following code is copy/pasted verbatim from a live production environment (the entire project/solution is on the production file system, side-by-side with the executing code):
Private Function GrabDataSet(ByVal sSQL As String) As DataSet
Try
Dim oDAdapt As SqlClient.SqlDataAdapter
Dim oRs As New DataSet
Dim cmdSQL As String = sSQL
oRs = New DataSet
oDAdapt = New SqlClient.SqlDataAdapter(cmdSQL, oConn)
oDAdapt.Fill(oRs)
Return oRs
oRs.Dispose()
oDAdapt.Dispose()
Catch ex As Exception
End Try
End Function
Found this in a web service I'm upgrading. It's hard to find anything NOT wrong with this.
|
|
|
|
|
It's funny that it's called GrabDataSet instead of GetDataSet. What are you gonna do? Squeeze it until the data flows out.
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]
|
|
|
|
|
I feel that "Strangle" would be the more appropriate verb.
|
|
|
|
|
He he he
Chris, The best comeback I have for that is "But ... Get did not work "
|
|
|
|
|
AspDotNetDev wrote: It's hard to find anything NOT wrong with this
M, actually no. I think there are things done right in there. What's wrong with this?
Dim oDAdapt As SqlClient.SqlDataAdapter
or this?
Dim cmdSQL As String = sSQL
And the author catches ALL exceptions, isn't that incredible?
And the user doesn't even know something bad occurred, how cool is that?
[Ok, ending sarcasm now ]
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
I'm always curious as to why people insist on declaring ALL variables inside a try/catch block. It's not as though declaring the first three variables there are likely to cause an exception (nor that instantiating the DataSet and SqlDataAdapter will cause problems).
|
|
|
|