|
From that, it look like a horror.
Did he comment it at all? Or just throw it together and walk away?
'Cause if he didn't, it's probably time he met Mr Ugly Stick...
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
That is definately an absolute horror. I hate it when people think they are smart because they write such code. It is totally unmaintainable and unreadable to anyone (including the author of it a couple of weeks after he wrote it!)
|
|
|
|
|
|
It's definitely a horror. It's unclear, poorly named and results in ugly calling code. In addition, the cast and call to a Convert function will make it slow.
|
|
|
|
|
What do you mean by "C# wouldn't allow abuse of the comma operator"? C# doesn't have a comma operator.
A quick search pulled up this little gem (I wonder how it got copied from the MSDN foum ): http://www.dotnetmonster.com/Uwe/Forum.aspx/dotnet-csharp/93126/C-for-loop-and-comma-operator[^]
Yes, that's a horror -- the method should have a better name and it could even be made generic!
public T
LastOf
(
params object[] list
)
{
return ( (T) list [ list.Count - 1 ] ) ;
}
It's still silly, but this just points up how easy it is to implement a way to get around the limitation. Come to think of it, couldn't you do it with Linq? I don't know Linq, but something like: (new int[] { a++ , b++ }).Last ?
P.S. Here's a little test of the concept:
int i = 0 ;
int j = 0 ;
while ( (new int[] { i++ , j += 2 }).Last() < 10 )
{
System.Console.WriteLine ( "{0} {1}" , i , j ) ;
}
for ( i = 0 , j = 0 ; (new int[] { i++ , j += 2 }).Last() < 10 ; )
{
System.Console.WriteLine ( "{0} {1}" , i , j ) ;
}
modified 7-Aug-12 18:09pm.
|
|
|
|
|
Apparently no, isnt't it? But run this code:
class Program {
static void Main(string[] args) {
object o = null;
Console.WriteLine(o.IsNull());
}
}
public static class ExtensionMethods {
public static bool IsNull(this object obj) {
return (obj == null);
}
}
If you expected it to throw a NullReferenceException (as I did), then you're in for a surprise. It actually prints true. Makes me wonder how extension methods are implemented internally. Something like a wrapper object over the null object? Any .NET experts here that can explain?
|
|
|
|
|
Extension methods are implemented as you would expect: as a call to a static method, with a parameter.
So effectively, what you have written is:
public static bool IsNull(object o)
{
return (o == null);
}
...
if (IsNull(null))
{
... Which nobody would expect to throw a null reference.
The extension method bit is just syntactic sugar.
Looks odd though!
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
Yes, I understand. But what intrigues me is the
o.IsNull() syntax, the mental picture I get is that since o is null, any attempt to call a method/field would throw a NullReferenceException, replace IsNull() with ToString() and you'll know what I'm talking about.
object.IsNull(o) would have made much sense.
|
|
|
|
|
The whole point of extension methods is to avoid having to make explicit static calls.
I don't really like them because they're hiding what's actually going on, but on the other hand those Linq method trains on IEnumerable<T> (all extension methods obviously) are really convenient.
I don't get the point of IsNull, though. o.IsNull() is more characters and less clear than o == null! (And although you can override == on a custom type so that does something different, anyone who overrides it in a way that makes o == null not do what you expect should be taken out and shot repeatedly.)
|
|
|
|
|
Or be burned alive. In hot lava. Along with the code he wrote.
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[ ^]
|
|
|
|
|
I often use this as an 'exploit', can help make your code more readable in some cases. Sometimes I rather create an extension method than an instance method and do the null checking in the method instead of the calling procedure. (Not recommended as default pattern though)
____________________________________________________________
Be brave little warrior, be VERY brave
|
|
|
|
|
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.
|
|
|
|