Why aren't we using idiomatic C#?

All over the code, Sam uses methods instead of properties, where the latter would be more appropriate, and in this lecture, he chose a really convoluted approach to sorting the hits where this would have sufficed:

RaycastHit[] hits = Physics.RaycastAll(MouseRay);
Array.Sort(hits, (a, b) => Mathf.RoundToInt(a.distance - b.distance));

Is this some decision made for compatibility reasons? The latest versions of Unity allow the use of C#7 (8 is here, as of now, but 7 does well too).

PS the only notable exception is within Sam’s own LazyValue class.

1 Like

Sam says mid-lecture that it was being done this way as it avoided introducing too much new or different language features, whilst acknowledging there are several ways it could have been addressed.

Sure they could have played hard ball and made every lecture deliver every concept using whatever happened to be the most efficient and modern coding technique, but they haven’t and I suspect it’s to keep the material approachable for those who are not C# aficionados.

Furthermore, your code seems to not work. You’re required to return an int, but that should a representative value of the compare result in accordance with this:

https://docs.microsoft.com/en-us/dotnet/api/system.collections.icomparer.compare?view=netframework-4.8

Instead in your case, if items are no more than 50cm apart they could be returned in the wrong order depending on where they appear in the array, which is a bug.

Give the following code a spin over at https://dotnetfiddle.net and see the difference between your implementation and how the results of the compare should be returned:

using System;

public struct Hits {
	public int index;
	public float distance;
}

public class Program
{
	static Hits[] hitsDemo1;
	static Hits[] hitsDemo2;
	
	public static void Main()
	{
		PopulateArrays();
		BadSort();
		DisplayFirst("Bad");
		
		PopulateArrays();
		GoodSort();
		DisplayFirst("Good");
	}

	private static void DisplayFirst(string header) {
		Console.WriteLine(string.Format("----------- {0} Results -------------", header));
		Console.WriteLine(hitsDemo1[0].index);
		Console.WriteLine(hitsDemo2[0].index);
	}
	
	private static void BadSort() {
		Array.Sort(hitsDemo1, (a, b) => (int)Math.Round(a.distance - b.distance));
		Array.Sort(hitsDemo2, (a, b) => (int)Math.Round(a.distance - b.distance));
	}
	
	private static void GoodSort() {
		Array.Sort(hitsDemo1, (a, b) => Math.Abs(a.distance - b.distance) <= float.Epsilon ? 0 : a.distance > b.distance ? 1 : -1);
		Array.Sort(hitsDemo2, (a, b) => Math.Abs(a.distance - b.distance) <= float.Epsilon ? 0 : a.distance > b.distance ? 1 : -1);
	}
	
	private static void PopulateArrays() {
		hitsDemo1 = new Hits[] { 
			new Hits { index = 1, distance = 3f},
			new Hits { index = 2, distance = 3.3f},
			new Hits { index = 3, distance = 3.8f}
		};
		
		hitsDemo2 = new Hits[] { 
			new Hits { index = 2, distance = 3.3f},
			new Hits { index = 3, distance = 3.8f},
			new Hits { index = 1, distance = 3f}
		};
	}
}

So maybe not a bad idea to stick with something simpler at this point :wink:

Edit: Initially I said to use Roslyn 2.0 compiler on .net fiddle but that seems slow/buggy today, so changed the code a little to compile under any of their options.

2 Likes

You’re absolutely right about the catch on the 50cm stuff - absolutely didn’t notice this. Still, for the top down perspective I don’t mind it. I’ll fix my code to return a unit result, I guess - but it’s dubiously idiot-proof so it works in my case. :joy:

I’ve already gone down the path of C#7. There’s no turning back for me, now. :grimacing:

Check out https://gitlab.com/ShivamMukherjee/TL7_RPG2 - I’ve essentially done a lot of things differently.

1 Like

This topic was automatically closed 24 hours after the last reply. New replies are no longer allowed.

Privacy & Terms