Mfcttrf

This is part of a game made in Unity3D.

I wrote this class to convert geographic coordinates into lambert 2008 coordinates based on this reference document:

http://www.ngi.be/Common/Lambert2008/Transformation_Geographic_Lambert_FR.pdf

Reason is that I'm using a Belgian National Geographic Institute map as background, and want to locate waypoint and player position with full accuracy.

Code works fine, and runs in .000001s but I'm unhappy with its readability.

I start by declaring all the constants I need

public const int a = 6378137;
public const double f = 1 / 298.257222101;
public const double lowerLatitude = 49.8333333 * Mathf.Deg2Rad;
public const double upperLatitude = 51.1666667 * Mathf.Deg2Rad;
public const double originLatitude = 50.797815 * Mathf.Deg2Rad;
public const double originLongitude = 4.359215833 * Mathf.Deg2Rad;
public const int originX = 649328;
public const int originY = 665262;

Then I used properties for all values derived from those constants

public static double Excentricity get => System.Math.Sqrt((2 * f) - (f * f));

static double MLower get => M(lowerLatitude);
static double MUpper get => M(upperLatitude);

static double M(double latitude)

 return System.Math.Cos(latitude) / System.Math.Sqrt(1 - (Excentricity * Excentricity * System.Math.Pow(System.Math.Sin(latitude), 2)));


static double TLower get => T(lowerLatitude); 
static double TUpper get => T(upperLatitude); 
static double TOrigin get => T(originLatitude); 

static double T(double latitude)


 return System.Math.Tan(Mathf.PI / 4 - latitude / 2) / System.Math.Pow((1 - Excentricity * System.Math.Sin(latitude)) / (1 + Excentricity * System.Math.Sin(latitude)), Excentricity / 2);


static double N get => (System.Math.Log(MLower) - System.Math.Log(MUpper)) / (System.Math.Log(TLower) - System.Math.Log(TUpper)); 

static double G get => MLower / (N * System.Math.Pow(TLower, N)); 

static double ROrigin get => R(TOrigin); 

static double R (double t)

 return a * G * System.Math.Pow(t, N);

As you can see some of the calculations are written on very long lines. I'm unhappy with their readability, but I have no Idea on how to re-write them to make them look better while not messing up floating point approximations.

Finally I perform the actual conversion using all those calculated values.

public static Vector2 FromGeographicRelative(Vector2 coordinates)


 double t = T((double)coordinates.x * Mathf.Deg2Rad);
 double r = R(t);
 double angle = N * ((double)coordinates.y * Mathf.Deg2Rad - originLongitude);
 Vector2 lambertCoord = new Vector2
 
 x = (float) (r * System.Math.Sin(angle)),
 y = (float) (ROrigin - r * System.Math.Cos(angle))
 ;
 return lambertCoord;

Since the code is about implementing mathematical formulas I would make an exception and completely ignore the usual naming conventions. Instead, I would use the exact symbols as that document.

This would mean a little bit copy/paste because it's difficult to write symbols like [ϕ, λ, θ] but the compiler can work with unicode.

I find that code like this:

var rad = 0.01745329252;

var ellipsoide = (a: 6378137, f: 1 / 298.257222101);
var ϕ1 = 49.8333333 * rad;
var ϕ2 = 51.1666667 * rad;
var ϕ0 = 50.797815 * rad;
var λ0 = 4.359215833 * rad;
var x0 = 649328;
var y0 = 665262;

is the most easy one to understand because you can compare it with the book almost without any translation.

I think it's a code smell to have more than one (infix) operator in one line.

What would be the recommended way of doing this, both for readability and performance?

Yes! But please use good names.

For performance it doesn't matter. The C# compiler will probably inline those things so it's the same generated IL code

Related:

"Expressions should not be too complex"

https://rules.sonarsource.com/csharp/RSPEC-1067?search=operator

Noncompliant Code Example
With the default threshold value of 3

if (((condition1 && condition2) || (condition3 && condition4)) && condition5) ... 

Compliant Solution

if ((MyFirstCondition() || MySecondCondition()) && MyLastCondition()) ... 

• In math heavy code you might use the using static directive, see the microsoft documentation such that System.Math. can be removed throughout the code.


• 
  Excentricity can be made a constant and may have a shorter name as I have seen in other codes dealing with WGS84.


• If possible refer to a document and formula number in a comment


• Although sometime frowned on, I like to layout the the code as much as possible as the mathematical equation looks like. (use extra parentheses if it improves readability)

With this in mind I came to to following example for method T.

using static System.Math;

public const double E2 = (2 * f) - (f * f); # Excentricity squared
public const double E1 = Sqrt(E2); # Excentricity

.....

 static double T(double lat)
 
 // if possible refer to a document and formula number
 return Tan(PI / 4 - lat / 2) 
 / 
 Pow( ((1 - E1 * Sin(lat)) 
 / 
 (1 + E1 * Sin(lat))) , E1 / 2);
 

Naming Conventions

Some, but not all of your variables and functions are named after mathematical usage. I like it either way, but a combination is an inconsistent naming convention.

public const double f = 1 / 298.257222101;
public const double lowerLatitude = 49.8333333 * Mathf.Deg2Rad;

Constants

You have a lot of mathematical constants that aren't constants in your API. Why?

Excentricity, MLower, MUpper, TLower, TUpper, TOrigin, ROrigin, N, G.

Aliases

You are using both System.Math and UnityEngine.Mathf. To avoid repeating code I suggest to import the one you used most as a static using and the other as an aliased using.

using static UnityEngine.Mathf;
using Math = System.Math;