polish: take in account code review

flyingtof · flyingtof · commit 94322bb804db · 2024-05-28T18:55:35.000+02:00
diff --git a/apps/fxc-front/src/app/components/2d/path-element.ts b/apps/fxc-front/src/app/components/2d/path-element.ts
@@ -13,12 +13,12 @@ import { FaiSectors } from '../../gm/fai-sectors';
 import { addAltitude } from '../../logic/elevation';
 import { getCurrentUrl, pushCurrentState } from '../../logic/history';
 import { drawRoute } from '../../logic/messages';
-import { CircuitType, Score } from '../../logic/score/scorer';
+import { CircuitType, getCircuitType, Score } from '../../logic/score/scorer';
 import { setDistance, setEnabled, setRoute, setScore } from '../../redux/planner-slice';
 import { RootState, store } from '../../redux/store';
 import { PlannerElement } from './planner-element';
-import { OptimizationResult, optimize, ScoringRules, ScoringTrack } from 'optimizer';
-import { LeagueCode } from '../../logic/score/league/leagues';
+import { getOptimizer, ScoringTrack } from 'optimizer';
+import { getScoringRules } from '../../logic/score/league/leagues';
 
 // Route color by circuit type.
 const ROUTE_STROKE_COLORS = {
@@ -225,10 +225,10 @@ export class PathElement extends connect(store)(LitElement) {
       this.closingSector.addListener('rightclick', (e) => this.appendToPath(e.latLng));
     }
 
-    if (score.closingRadius) {
+    if (score.closingRadiusM) {
       const center = points[score.indexes[0]];
       this.closingSector.center = center;
-      this.closingSector.radius = score.closingRadius;
+      this.closingSector.radius = score.closingRadiusM;
       this.closingSector.update();
       this.closingSector.setMap(this.map);
     } else {
@@ -252,67 +252,26 @@ export class PathElement extends connect(store)(LitElement) {
 
   private computeScore(points: LatLon[]): Score {
     const track: ScoringTrack = {
-      points: points.map((point, i) => {
-        return {
-          ...point,
-          alt: 0,
-          timeSec: i * 60,
-        };
-      }),
-      minTimeSec: new Date().getTime() / 1000,
+      points: points.map((point, i) => ({ ...point, alt: 0, timeSec: i * 60 })),
+      startTimeSec: new Date().getTime() / 1000,
     };
-    const result = optimize({ track }, this.getLeague()).next().value;
-    const score = new Score({
-      circuit: this.getCircuitType(result),
-      distance: result.lengthKm * 1000,
+    const result = getOptimizer({ track }, getScoringRules(this.league)).next().value;
+    return new Score({
+      circuit: getCircuitType(result.circuit),
+      distanceM: result.lengthKm * 1000,
       multiplier: result.multiplier,
-      closingRadius: result.closingRadius ? result.closingRadius * 1000 : null,
+      closingRadiusM: result.closingRadius ? result.closingRadius * 1000 : null,
       indexes: result.solutionIndices,
+      points: result.score,
     });
-    // force the score as computed because of an unwanted side effect in constructor.
-    score.forcePoints(result.score);
-    return score;
-  }
-
-  private getLeague(): ScoringRules {
-    switch (this.league as LeagueCode) {
-      case 'czl':
-        return ScoringRules.CzechLocal;
-      case 'cze':
-        return ScoringRules.CzechEuropean;
-      case 'czo':
-        return ScoringRules.CzechOutsideEurope;
-      case 'fr':
-        return ScoringRules.FederationFrancaiseVolLibre;
-      case 'leo':
-        return ScoringRules.Leonardo;
-      case 'nor':
-        return ScoringRules.Norway;
-      case 'ukc':
-        return ScoringRules.UnitedKingdomClub;
-      case 'uki':
-        return ScoringRules.UnitedKingdomInternational;
-      case 'ukn':
-        return ScoringRules.UnitedKingdomNational;
-      case 'xc':
-        return ScoringRules.XContest;
-      case 'xcppg':
-        return ScoringRules.XContestPPG;
-      case 'wxc':
-        return ScoringRules.WorldXC;
-    }
-  }
-
-  private getCircuitType(result: OptimizationResult) {
-    return result.circuit as unknown as CircuitType;
   }
 
   // Sends a message to the iframe host with the changes.
   private postScoreToHost(score: Score) {
     let kms = '';
     let circuit = '';
-    if (score.distance && window.parent) {
-      kms = (score.distance / 1000).toFixed(1);
+    if (score.distanceM && window.parent) {
+      kms = (score.distanceM / 1000).toFixed(1);
       circuit = CIRCUIT_SHORT_NAME[score.circuit];
       if (score.circuit == CircuitType.OpenDistance) {
         circuit += score.indexes.length - 2;
diff --git a/apps/fxc-front/src/app/components/2d/planner-element.ts b/apps/fxc-front/src/app/components/2d/planner-element.ts
@@ -139,7 +139,7 @@ export class PlannerElement extends connect(store)(LitElement) {
         <div>
           <div>${this.score.circuit}</div>
           <div class="large">
-            ${unsafeHTML(units.formatUnit(this.score.distance / 1000, this.units.distance, undefined, 'unit'))}
+            ${unsafeHTML(units.formatUnit(this.score.distanceM / 1000, this.units.distance, undefined, 'unit'))}
           </div>
         </div>
         <div class="collapsible">
diff --git a/apps/fxc-front/src/app/logic/score/league/leagues.ts b/apps/fxc-front/src/app/logic/score/league/leagues.ts
@@ -1,4 +1,10 @@
-export const LEAGUES: { [name: string]: string } = {
+import { ScoringRules } from 'optimizer';
+
+// allowed league codes
+export const leagueCodes = ['czl', 'cze', 'czo', 'fr', 'leo', 'nor', 'ukc', 'uki', 'ukn', 'xc', 'xcppg', 'wxc'];
+export type LeagueCode = (typeof leagueCodes)[number];
+
+export const LEAGUES: Readonly<Record<LeagueCode, string>> = {
   czl: 'Czech (ČPP local)',
   cze: 'Czech (ČPP Europe)',
   czo: 'Czech (ČPP outside Europe)',
@@ -13,7 +19,32 @@ export const LEAGUES: { [name: string]: string } = {
   wxc: 'World XC Online Contest',
 };
 
-// allowed league codes
-// ensure that all league codes defined in each League sub classes are in this
-// closed set.
-export type LeagueCode = 'czl' | 'cze' | 'czo' | 'fr' | 'leo' | 'nor' | 'ukc' | 'uki' | 'ukn' | 'xc' | 'xcppg' | 'wxc';
+export function getScoringRules(league: string): ScoringRules {
+  switch (league) {
+    case 'czl':
+      return ScoringRules.CzechLocal;
+    case 'cze':
+      return ScoringRules.CzechEuropean;
+    case 'czo':
+      return ScoringRules.CzechOutsideEurope;
+    case 'fr':
+      return ScoringRules.FederationFrancaiseVolLibre;
+    case 'leo':
+      return ScoringRules.Leonardo;
+    case 'nor':
+      return ScoringRules.Norway;
+    case 'ukc':
+      return ScoringRules.UnitedKingdomClub;
+    case 'uki':
+      return ScoringRules.UnitedKingdomInternational;
+    case 'ukn':
+      return ScoringRules.UnitedKingdomNational;
+    case 'xc':
+      return ScoringRules.XContest;
+    case 'xcppg':
+      return ScoringRules.XContestPPG;
+    case 'wxc':
+      return ScoringRules.WorldXC;
+  }
+  throw Error('no corresponding rule for ' + league);
+}
diff --git a/apps/fxc-front/src/app/logic/score/scorer.ts b/apps/fxc-front/src/app/logic/score/scorer.ts
@@ -1,27 +1,31 @@
+import { CircuitType as OptimizerCircuitType } from 'optimizer';
+
 export enum CircuitType {
   OpenDistance = 'Open distance',
   FlatTriangle = 'Flat triangle',
   FaiTriangle = 'Fai triangle',
   OutAndReturn = 'Out and return',
 }
 
+export function getCircuitType(circuit?: OptimizerCircuitType) {
+  return circuit as unknown as CircuitType;
+}
+
+
 export class Score {
-  distance: number;
+  distanceM: number;
   indexes: number[];
   multiplier: number;
   circuit: CircuitType;
-  closingRadius: number | null;
+  closingRadiusM: number | null;
   points: number;
 
-  constructor(score: Partial<Omit<Score,'points'>>) {
-    this.distance = score.distance || 0;
-    this.indexes = score.indexes || [];
-    this.multiplier = score.multiplier || 1;
-    this.circuit = score.circuit || CircuitType.OpenDistance;
-    this.closingRadius = score.closingRadius || null;
-    this.points = (this.distance * this.multiplier) / 1000;
-  }
-  public forcePoints(points: number){
-    this.points = points;
+  constructor(score: Partial<Score>) {
+    this.distanceM = score.distanceM ?? 0;
+    this.indexes = score.indexes ?? [];
+    this.multiplier = score.multiplier ?? 1;
+    this.circuit = score.circuit ?? CircuitType.OpenDistance;
+    this.closingRadiusM = score.closingRadiusM ?? null;
+    this.points = score.points ?? 0;
   }
 }
diff --git a/libs/optimizer/src/index.ts b/libs/optimizer/src/index.ts
@@ -1,7 +1,7 @@
-export { optimize } from './lib/optimizer';
+export { getOptimizer } from './lib/optimizer';
 export type {
   LatLonAltTime,
-  OptimizedCircuitType,
+  CircuitType,
   ScoringTrack,
   OptimizationResult,
   OptimizationOptions,
diff --git a/libs/optimizer/src/lib/fixtures/optimizer.fixtures.ts b/libs/optimizer/src/lib/fixtures/optimizer.fixtures.ts
@@ -1,4 +1,4 @@
-import { OptimizationRequest, OptimizationResult, OptimizedCircuitType } from '../optimizer';
+import { OptimizationRequest, OptimizationResult, CircuitType } from '../optimizer';
 import { computeDestinationPoint, getGreatCircleBearing, getPreciseDistance } from 'geolib';
 import { createSegments } from '../utils/createSegments';
 import { concatTracks } from '../utils/concatTracks';
@@ -25,7 +25,7 @@ export type OptimizerFixture = {
 export function createEmptyTrackFixture(): OptimizerFixture {
   return {
     givenRequest: {
-      track: { points: [], minTimeSec: 0 },
+      track: { points: [], startTimeSec: 0 },
     },
     givenRules: ScoringRules.FederationFrancaiseVolLibre,
     expectedResult: {
@@ -73,7 +73,7 @@ export function createFreeDistanceFixture(
       score: distance * multiplier,
       lengthKm: distance,
       multiplier,
-      circuit: OptimizedCircuitType.OpenDistance,
+      circuit: CircuitType.OpenDistance,
       optimal: true,
     },
   };
@@ -119,7 +119,7 @@ export function createFreeDistance1PointFixture(
       score: distance * multiplier,
       lengthKm: distance,
       multiplier,
-      circuit: OptimizedCircuitType.OpenDistance,
+      circuit: CircuitType.OpenDistance,
       optimal: true,
     },
   };
@@ -177,7 +177,7 @@ export function createFreeDistance2PointsFixture(
       score: distance * multiplier,
       lengthKm: distance,
       multiplier,
-      circuit: OptimizedCircuitType.OpenDistance,
+      circuit: CircuitType.OpenDistance,
       optimal: true,
     },
   };
@@ -244,7 +244,7 @@ export function createFreeDistance3PointsFixture(
       score: distance * multiplier,
       lengthKm: distance,
       multiplier,
-      circuit: OptimizedCircuitType.OpenDistance,
+      circuit: CircuitType.OpenDistance,
       optimal: true,
     },
   };
@@ -270,7 +270,7 @@ export function createClosedFlatTriangleFixture(
     throw new Error('invalid test data: not a flat triangle');
   }
   const multiplier = getFlatTriangleMultiplier(givenRules);
-  return createTriangleFixture(start, p1, p2, nbSegments, givenRules, multiplier, OptimizedCircuitType.FlatTriangle);
+  return createTriangleFixture(start, p1, p2, nbSegments, givenRules, multiplier, CircuitType.FlatTriangle);
 }
 
 /**
@@ -298,7 +298,7 @@ export function createClosedFaiTriangleFixture(
     throw new Error('invalid test data: not a FAI triangle');
   }
   const multiplier = getFaiTriangleMultiplier(givenRules);
-  return createTriangleFixture(start, p1, p2, nbSegments, givenRules, multiplier, OptimizedCircuitType.FaiTriangle);
+  return createTriangleFixture(start, p1, p2, nbSegments, givenRules, multiplier, CircuitType.FaiTriangle);
 }
 
 /**
@@ -430,7 +430,7 @@ function createTriangleFixture(
   nbSegments: number,
   givenRules: ScoringRules,
   multiplier: number,
-  circuit: OptimizedCircuitType,
+  circuit: CircuitType,
 ): OptimizerFixture {
   const distance1 = getPreciseDistance(start, p1);
   const distance2 = getPreciseDistance(p1, p2);
diff --git a/libs/optimizer/src/lib/optimizer.spec.ts b/libs/optimizer/src/lib/optimizer.spec.ts
@@ -1,4 +1,4 @@
-import { OptimizationResult, optimize } from './optimizer';
+import { OptimizationResult, getOptimizer } from './optimizer';
 import {
   createClosedFaiTriangleFixture,
   createClosedFaiTriangleFixtureWithSmallCycle,
@@ -34,11 +34,11 @@ describe('optimizer', () => {
     ScoringRules.XContestPPG,
     ScoringRules.WorldXC,
   ].forEach((rules) => {
-    describe(ScoringRules[rules] + ' rules', () => {
+    describe(`${ScoringRules[rules]} rules`, () => {
       const oneSegmentPerBranch = 1;
       const tenSegmentsPerBranch = 10;
       [oneSegmentPerBranch, tenSegmentsPerBranch].forEach((nbSegmentsPerBranch) => {
-        describe('given a free distance request (' + nbSegmentsPerBranch + ' segments(s)/branch)', () => {
+        describe(`given a free distance request (${nbSegmentsPerBranch} segments(s)/branch)`, () => {
           const fixture = createFreeDistanceFixture(
             { lat: 45, lon: 5 },
             { lat: 45, lon: 6 },
@@ -51,7 +51,7 @@ describe('optimizer', () => {
         });
 
         describe(
-          'given a free distance with 1 intermediate point request (' + nbSegmentsPerBranch + ' segment(s)/branch)',
+          `given a free distance with 1 intermediate point request (${nbSegmentsPerBranch} segment(s)/branch)`,
           () => {
             const fixture = createFreeDistance1PointFixture(
               { lat: 45, lon: 5 },
@@ -67,7 +67,7 @@ describe('optimizer', () => {
         );
 
         describe(
-          'given a free distance with 2 intermediate points request (' + nbSegmentsPerBranch + ' segment(s)/branch)',
+          `given a free distance with 2 intermediate points request (${nbSegmentsPerBranch} segment(s)/branch)`,
           () => {
             const fixture = createFreeDistance2PointsFixture(
               { lat: 45, lon: 5 },
@@ -84,7 +84,7 @@ describe('optimizer', () => {
         );
 
         describe(
-          'given a free distance with 3 intermediate points request (' + nbSegmentsPerBranch + ' segment(s)/branch)',
+          `given a free distance with 3 intermediate points request (${nbSegmentsPerBranch} segment(s)/branch)`,
           () => {
             const fixture = createFreeDistance3PointsFixture(
               { lat: 45, lon: 5 },
@@ -101,7 +101,7 @@ describe('optimizer', () => {
           },
         );
 
-        describe('given a closed flat triangle request (' + nbSegmentsPerBranch + ' segment(s)/branch)', () => {
+        describe(`given a closed flat triangle request (${nbSegmentsPerBranch} segment(s)/branch)`, () => {
           const fixture = createClosedFlatTriangleFixture(
             { lat: 45, lon: 5 },
             { lat: 45, lon: 6 },
@@ -114,7 +114,7 @@ describe('optimizer', () => {
           });
         });
 
-        describe('given a closed FAI triangle request (' + nbSegmentsPerBranch + ' segment(s)/branch)', () => {
+        describe(`given a closed FAI triangle request (${nbSegmentsPerBranch} segment(s)/branch)`, () => {
           const fixture = createClosedFaiTriangleFixture(
             { lat: 45, lon: 5 },
             { lat: 45, lon: 6 },
@@ -153,8 +153,10 @@ describe('optimizer', () => {
     });
   });
 
+  // TODO: IsAsExpected does not really describe the behavior. Something with expect(optimize(...)).toHaveScore(...);
+  // should be better
   function expectOptimizationIsAsExpected(fixture: OptimizerFixture) {
-    const optimization = optimize(fixture.givenRequest, fixture.givenRules);
+    const optimization = getOptimizer(fixture.givenRequest, fixture.givenRules);
     let currentResult: IteratorResult<OptimizationResult, OptimizationResult>,
       done = false;
     while (!done) {
diff --git a/libs/optimizer/src/lib/optimizer.ts b/libs/optimizer/src/lib/optimizer.ts
diff --git a/libs/optimizer/src/lib/utils/concatTracks.ts b/libs/optimizer/src/lib/utils/concatTracks.ts
diff --git a/libs/optimizer/src/lib/utils/createSegments.ts b/libs/optimizer/src/lib/utils/createSegments.ts
