comparing two names using fuzzy

Question

I am joining a new company and I want to make a positive impact in regards to my code.

I was asked to do compare two fuzzy name and return if the name is valid or not.

For this I wrote the following code

import fuzzy from 'fuzzy'



const {driverName, tlcNumber} = input

    if (!tlcNumber) {
      throw new Error("tlcNumber not specified");
    } 
    if (!driverName) {
        throw new Error("Driver Number not specified");
    }
    const result = await getDriver(input.tlcNumber);
    if (!result) {
        return {
            valid: false, 
            message: `TLC number ${tlcNumber} is not found in the TLC database`
        }
    }
    const {name:tlcRegisteredName} = result

    driverName.split(' ').forEach(name => {
      const fuzzyMatch = fuzzy.test(name, tlcRegisteredName)
      if (!fuzzyMatch) {
        return {
          valid: false, 
          message: `TLC number ${tlcNumber} is registered to ${tlcRegisteredName} which does not match ${driverName}`
        }
      }
    });

    return {
      valid:true
    }

Can someone help me in making this code better? or how I would Improve? I have been told to

separate first and last names and do fuzzy matching independently on each

I am using the fuzzy library.

Answer 1

I guess this code was taken a bit out of context, and might reside in a different form elsewhere. I've noticed a few things here, so here is my review:

1. Everything resides in global space and the execution not really structured. Each step happens in a sequential manner, where the actual description on what happens has to be understood by looking at the sequence as a whole. Meaning, extract piece of code into (pure) functions - with proper naming.
2. For software in terms of maintainability and readability it's important to be consistent even when/especially syntax wise. For example, there is inconsistent usage of semicola and access to variables is also not consistent e.g. tlcNumber vs. input.tlcNumber
3. Extract constant wherever possible, for example .split(' '), why is it an empty string? Is this some sort of name separator?
4. Splitting the drivers name and checking whether or not a substring of the name matches in order to return a check result could be refactored. In fact, TypeScript Playground even yelled at me, that not all codepaths return a value. Consider using functions like, .map(), .some() to achieve the same. The approach is totally fine when for example, an early stop of execution is mandatory.
5. In general, it might be helpful to avoid abbreviations for variables as they might cause confusion. My personal preference is to use longer and self-explanatory variable names rather than some abbreviation which is not common sense.

As well I do have questions looking at the code, some of them are:

1. What is input where does it come from? Which typings does it have?
2. Should this whole code be some sort of API which another person relies on? For example doesDriverNameAndTlcDriverNameMatch({driverName: string; tlcNumber: number });

So, my refactored version of the above code would look like this:

import fuzzy from "fuzzy";

type DriverNameAndTlcNumber = {
  driverName: string;
  tlcNumber: number;
}

type PositiveMatch = {
  valid: true
}
type NegativeMatch = {
  valid: false;
  message: string;
}
type MatchResult = PositiveMatch | NegativeMatch;

const NAME_SEPARATOR = ' ';

const getDriverByTlcNumber = async (driversTlcNumber: number): Promise<{name: string;}> => await getDriver(driversTlcNumber);

export const doesDriverNameAndTlcDriverNameMatch = async ({driverName, tlcNumber}: DriverNameAndTlcNumber): Promise<MatchResult> => {
  if (!tlcNumber) {
    throw new Error("tlcNumber not specified");
  } 
  if (!driverName) {
    throw new Error("Driver Number not specified");
  }

  const driver = await getDriverByTlcNumber(tlcNumber);

  if (!driver) {
    return {valid: false, message: `TLC number ${tlcNumber} is not found in the TLC database`};
  }

  const driverNameAndRegisteredNameDoNotMatch = driver.name.split(NAME_SEPARATOR)
            .map(x => fuzzy.test(x, driver.name))
            .some(x => x === undefined);

  return driverNameAndRegisteredNameDoNotMatch ? { valid: false, message: `TLC number ${tlcNumber} is registered to ${driver.name} which does not match ${driverName}` } : { valid: true };
};