From 8cd8501d7a240282f342950265216c83a840efb8 Mon Sep 17 00:00:00 2001
From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Date: Thu, 27 Jul 2023 08:36:31 -0700
Subject: [PATCH] Refactoring.

---
 .../sdk-server/src/MigrationOpTracker.ts      | 90 +++++++++++--------
 1 file changed, 53 insertions(+), 37 deletions(-)

diff --git a/packages/shared/sdk-server/src/MigrationOpTracker.ts b/packages/shared/sdk-server/src/MigrationOpTracker.ts
index e517e207a..012487cc0 100644
--- a/packages/shared/sdk-server/src/MigrationOpTracker.ts
+++ b/packages/shared/sdk-server/src/MigrationOpTracker.ts
@@ -8,6 +8,10 @@ import {
 } from './api/data';
 import { LDMigrationOrigin } from './api/LDMigration';
 
+function isPopulated(data: number): boolean {
+  return !Number.isNaN(data);
+}
+
 export default class MigrationOpTracker implements LDMigrationTracker {
   private errors = {
     old: false,
@@ -65,43 +69,11 @@ export default class MigrationOpTracker implements LDMigrationTracker {
   createEvent(): LDMigrationOpEvent | undefined {
     if (this.operation && this.context.valid) {
       const measurements: LDMigrationMeasurement[] = [];
-      if (
-        // Cannot use a truthy check as 0 is a desired value.
-        this.consistencyCheck !== undefined &&
-        this.consistencyCheck !== LDConsistencyCheck.NotChecked
-      ) {
-        measurements.push({
-          key: 'consistent',
-          value: this.consistencyCheck,
-          // TODO: Needs to come from someplace.
-          samplingOdds: 0,
-        });
-      }
-      if (
-        !Number.isNaN(this.latencyMeasurement.new) ||
-        !Number.isNaN(this.latencyMeasurement.old)
-      ) {
-        const values: { old?: number; new?: number } = {};
-        if (!Number.isNaN(this.latencyMeasurement.new)) {
-          values.new = this.latencyMeasurement.new;
-        }
-        if (!Number.isNaN(this.latencyMeasurement.old)) {
-          values.old = this.latencyMeasurement.old;
-        }
-        measurements.push({
-          key: 'latency',
-          values,
-        });
-      }
-      if (this.errors.new || this.errors.old) {
-        measurements.push({
-          key: 'error',
-          values: {
-            old: this.errors.old ? 1 : 0,
-            new: this.errors.new ? 1 : 0,
-          },
-        });
-      }
+
+      this.populateConsistency(measurements);
+      this.populateLatency(measurements);
+      this.populateErrors(measurements);
+
       return {
         kind: 'migration_op',
         operation: this.operation,
@@ -119,4 +91,48 @@ export default class MigrationOpTracker implements LDMigrationTracker {
     }
     return undefined;
   }
+
+  private populateConsistency(measurements: LDMigrationMeasurement[]) {
+    if (
+      this.consistencyCheck !== undefined &&
+      this.consistencyCheck !== LDConsistencyCheck.NotChecked
+    ) {
+      measurements.push({
+        key: 'consistent',
+        value: this.consistencyCheck,
+        // TODO: Needs to come from someplace.
+        samplingOdds: 0,
+      });
+    }
+  }
+
+  private populateErrors(measurements: LDMigrationMeasurement[]) {
+    if (this.errors.new || this.errors.old) {
+      measurements.push({
+        key: 'error',
+        values: {
+          old: this.errors.old ? 1 : 0,
+          new: this.errors.new ? 1 : 0,
+        },
+      });
+    }
+  }
+
+  private populateLatency(measurements: LDMigrationMeasurement[]) {
+    const newIsPopulated = isPopulated(this.latencyMeasurement.new);
+    const oldIsPopulated = isPopulated(this.latencyMeasurement.old);
+    if (newIsPopulated || oldIsPopulated) {
+      const values: { old?: number; new?: number } = {};
+      if (newIsPopulated) {
+        values.new = this.latencyMeasurement.new;
+      }
+      if (oldIsPopulated) {
+        values.old = this.latencyMeasurement.old;
+      }
+      measurements.push({
+        key: 'latency',
+        values,
+      });
+    }
+  }
 }