Jörgen Andersson (is this you "Neo"?!?) sounds scandinavian and they probably have a different decimal separator... Robert
Robert Rohde
Posts
-
Facepalm of the day. -
I just want to be really really sureNo it won't. It wouldn't compile...
-
The best lambda expression I ever sawWell that makes sense if someList is a list of objects which implement IEnumerable (like
List<List<int>>
. A bit weird but no horror. -
Handling "Special" Cases in "Special" Ways -
I understand that we shouldn't duplicate code, but...Quote:
If I was the manager over the person that did that, though, said person would be on the unemployment line or in a remedial C# class!
Don't be so fast with such statements. Have you tested your approach? I made a quick test and the params keyword introduces a really big performance penalty (more than factor 5, probably even 10). The reason is probably that for each call an array must be instantiated. If I was your manager, and you would implement it that way although it could be done more than 5 times faster, than you would probably be on the unemployment line ;). Also note that this functions are internal functions of the generic Tuple classes. Tuple has at most 8 generic type parameters (and as a consequence 8 properties where hash codes might get combined), so combining 9 hash codes is really not an issue here.
-
I understand that we shouldn't duplicate code, but...I really don't see why this should make a difference here, but to avoid a big discussion on how the JIT works I rewrote my tests. First I make one test run just for the JIT. Then I make 1000 test runs for each test case and in each test run 10.000.000 calls to CombineHashCodes are done. Here the results:
Test 1: -1607204864 Time Avg: 00:00:00.0281418 (100,00%)
Test 2: -1607204864 Time Avg: 00:00:00.0281436 (100,01%)
Test 3: -1607204864 Time Avg: 00:00:00.0312717 (111,12%)I feel the JIT optimizer does inline those nested calls exactly as I did. Here the complete code:
class Program
{
static void Main(string[] args)
{
var sw1 = new Stopwatch();
var sw2 = new Stopwatch();
var sw3 = new Stopwatch();
var res1 = 0;
var res2 = 0;
var res3 = 0;res1 = Test1(sw1, res1); res2 = Test2(sw2, res2); res3 = Test3(sw3, res3); //first run is for the JIT -> Reset stopwatches sw1.Reset(); sw2.Reset(); sw3.Reset(); int noOfRuns = 1000; for (int j = 0; j < noOfRuns; j++) { res1 = Test1(sw1, res1); res2 = Test2(sw2, res2); res3 = Test3(sw3, res3); } var avg1 = sw1.Elapsed.Ticks / noOfRuns; var avg2 = sw2.Elapsed.Ticks / noOfRuns; var avg3 = sw3.Elapsed.Ticks / noOfRuns; Console.WriteLine("Test 1: {0} Time Avg: {1} ({2:f2}%)", res1, new TimeSpan(avg1), 100.0); Console.WriteLine("Test 2: {0} Time Avg: {1} ({2:f2}%)", res2, new TimeSpan(avg2), avg2 \* 100.0 / avg1); Console.WriteLine("Test 3: {0} Time Avg: {1} ({2:f2}%)", res3, new TimeSpan(avg3), avg3 \* 100.0 / avg1); Console.ReadLine(); } private static int Test3(Stopwatch sw3, int res3) { res3 = 0; sw3.Start(); for (int i = 0; i < 10000000; i++) res3 += Tuple3.CombineHashCodes(i, i, i, i, i, i, i, i); sw3.Stop(); return res3; } private static int Test2(Stopwatch sw2, int res2) { res2 = 0; sw2.Start(); for (int i = 0; i < 10000000; i++) res2 += Tuple2.CombineHashCodes(i, i, i, i, i, i, i, i); sw2.Stop(); return res2; } private static int Test1(Stopwatch sw1, int res1) { res1 = 0; sw1.Start(); for (int i = 0; i < 10000000; i++)
-
I understand that we shouldn't duplicate code, but...1. The arithmetic overflow check is completely disabled in my solution (which ist the default setting; at least for VS 2010 console apps). Otherwise my whole test wouldn't work because I add so many large numbers that an OverflowException would occur. 2. Regarding the JIT: Thats why I made 3 (the for j loop) seperately stopped complete test runs. I always ignore the first result.
-
I understand that we shouldn't duplicate code, but...For the fun of it I tested your version against another version without the function calls. For this I just manually inlined all calls:
public static class Tuple2
{
internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5, int h6, int h7, int h8)
{
var h12 = (h1 << 5) + h1 ^ h2;
var h34 = (h3 << 5) + h3 ^ h4;
var h1234 = (h12 << 5) + h12 ^ h34;
var h56 = (h5 << 5) + h5 ^ h6;
var h78 = (h7 << 5) + h7 ^ h8;
var h5678 = (h56 << 5) + h56 ^ h78;
return (h1234 << 5) + h1234 ^ h5678;
}
}Just to be sure to optimize performance I even removed temp variables where possible (possible meaning "without having to calculate the same thing twice):
public static class Tuple3
{
internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5, int h6, int h7, int h8)
{
var h12 = (h1 << 5) + h1 ^ h2;
var h34 = (h3 << 5) + h3 ^ h4;
var h1234 = (h12 << 5) + h12 ^ h34;
var h56 = (h5 << 5) + h5 ^ h6;
return (h1234 << 5) + h1234 ^ ((h56 << 5) + h56 ^ ((h7 << 5) + h7 ^ h8));
}
}I tested your .NET disassembly code against this and the results show nearly no performance difference for my first alternative and only a minor (10-15% slower!!!) difference for the second one (can anybody tell me why this one is slower?!?). As the .NET code is clearly better readable and maintainable I would say they did everything correct. Here my test code:
static void Main(string[] args)
{
var sw1 = new Stopwatch();
var sw2 = new Stopwatch();
var sw3 = new Stopwatch();
var res = 0;for (int j = 0; j < 3; j++) { Console.WriteLine("{0}. run", j + 1); sw1.Reset(); res = 0; sw1.Start(); for (int i = 0; i < 10000000; i++) res += Tuple1.CombineHashCodes(i, i, i, i, i, i, i, i); sw1.Stop(); Console.WriteLine("Test 1: {0} Time: {1}", res, sw1.Elapsed); sw2.Reset(); res = 0; sw2.Start(); for (int i = 0; i < 10000000; i++) res += Tuple2.CombineHashCodes(i, i, i, i, i, i, i, i); sw2.St
-
Linq2HorrorRobCroll wrote:
Interesting, I actually prefer tabs.
I would say 2 developers have at least 3 opinions on that topic :) The screw up isn't that bad. At least not worse than before. Prior to our change the code gradually changed from tabs to spaces. Now we have one final commit which changes it all at once (about 1/3 of our codebase is affected). When viewing history, making diffs or merges we know the exact revision to exlude.
-
Linq2HorrorAHhhhhh, finally someone who feels with me and doesn't take it too serious. Thanks :)
-
Unsafe lockingHow about this:
public void OriginalMethod(bool requiresLocking, object otherParams)
{
if (requiresLocking)
{
lock (lockerObject)
{
LotsOfCodeMovedToNewMethod(otherParams);
}
}
else
{
LotsOfCodeMovedToNewMethod(otherParams);
}
} -
Linq2HorrorFrom my other post[^]:
Quote:
The code was executed exactly one time (ignoring test runs) and will never be touched again. Its nothing that will get checked in in production code or anything like that.
-
Linq2HorrorI should probably clarify why and how this code was created: We have a relatively large codebase. Some code files are old (C# 1.0 times) and contain tabs instead of white spaces (this was the default setting in VS2003 if I remember correctly). Now everytime we edit one of those files Visual Studio (now 2010) starts replacing the tabs automatically. This is simply annoying when using diffs and can be a complete pain when doing merges in our SVN. So we just had the idea to replace all left tabs with one big update. While discussing on how to do it was just an amusing idea to make it with Linq. The code was executed exactly one time (ignoring test runs) and will never be touched again. Its nothing that will get checked in in production code or anything like that. Robert
-
Linq2HorrorHi all, I think the following could also qualify as a coding horror but I intentionally wrote it this way (one giant linq statement) because of a discussion with a collegue:
Directory.EnumerateFiles(args[0], "*.cs", SearchOption.AllDirectories).AsParallel().Select(f => new { File = f, Bytes = File.ReadAllBytes(f) }).Select(f => new { File = f.File, Bytes = f.Bytes, Encoding = f.Bytes.Take(3).SequenceEqual(new byte[] { 239, 187, 191 }) ? Encoding.UTF8 : Encoding.Default }).ForAll(f => File.WriteAllText(f.File, f.Encoding.GetString(f.Bytes).Split(new string[] { Environment.NewLine }, StringSplitOptions.None).Select(l => new { Line = l, Index = l.Select((c, i) => new { Char = c, Index = i }).FirstOrDefault(c => c.Char != ' ' && c.Char != '\t') }).Select(l => l.Index == null ? l.Line : l.Line.Select((c, i) => (i < l.Index.Index && c == '\t') ? " " : c.ToString()).Aggregate((s1, s2) => s1 + s2)).Aggregate((s1, s2) => s1 + Environment.NewLine + s2), f.Encoding));
Here another version where I tried to get a proper formatting:
static void Main(string[] args)
{
Directory.EnumerateFiles(args[0], "*.cs", SearchOption.AllDirectories).
AsParallel().
Select(f => new {
File = f,
Bytes = File.ReadAllBytes(f)
}).
Select(f => new {
File = f.File,
Bytes = f.Bytes,
Encoding = f.Bytes.Take(3).SequenceEqual(new byte[] { 239, 187, 191 })
? Encoding.UTF8 : Encoding.Default
}).
ForAll(f => File.WriteAllText(f.File,
f.Encoding.GetString(f.Bytes).Split(
new string[] { Environment.NewLine }, StringSplitOptions.None).
Select(l => new { Line = l, Index = l.
Select((c, i) => new { Char = c, Index = i }).
FirstOrDefault(c => c.Char != ' ' && c.Char != '\t') }).
Select(l => l.Index == null ? l.Line : l.Line.
Select((c, i) => (i < l.Index.Index && c == '\t')
? " " : c.ToString()).Aggregate((s1, s2) => s1 + s2)).
Aggregate((s1, s2) => s1 + Environment.NewLine + s2), f.Encoding));
}So what does this do? . . . . . . . . . . . . . . . . . . . . . . . . args[0] should be a directory where C#-Files are located (*.cs). It opens all files, does a bit of
-
Gem... As time goes by..I agree that the original implementation is bad but beware that your fix could introduce some new issues. For example passing "2" will result in a different result than before ("AM" vs ""). This might even be Ok but passing "test" will generate an exception where the old implementation would just have returned an empty string. Robert
-
Stupid codeI don't know if this also applies to static properties but I had once a similar case in a normal instance property. The main issue for me was that Visual Studio (I think it was 2003) crashed when I tried to debug code where this class was used. Why? Because the debugger tried to show the Value in the Watch-window and couldn't handle the StackOverflowException.
-
Translating a jagged arrayI know but its so much fun... :-D
-
Translating a jagged arrayHow about this one?
public static T[][] Translate2<T>(this T[][] array)
{
if (array == null)
throw new ArgumentNullException();
if (array.Length == 0)
return new T[0][];return Enumerable.Range(0, array.Max(sub => sub == null ? 1 : sub.Length)). Select(i => Enumerable.Range(0, array.Length). Select(j => array\[j\] == null || i >= array\[j\].Length ? default(T) : array\[j\]\[i\]).ToArray()).ToArray();
}
Robert
-
SQL betweenOriginalGriff wrote:
in VB
DataRows(17)
could be an array element or a function call, in C# the difference is clearAs much as I prefer C# over VB.NET this is not true.
DataRows[17]
could either be an array access or a call to an indexer which is nothing else than a method call. Robert -
Mustang GTSounds incredible slow to me... I would probably fall asleep :laugh: