TL;DR Find the first non-undefined value in an array

This post does go on a bit, and I’m suprised that one small function written by another developer sparked all of this!! At first I was just looking to refactor the code to make it clean, but then I started to chase the abstraction dragon, read on to find out what happened.

Sometimes the data we have to work with is NOT good, and with the api I’m working with, that is a gross understatement. This is just one of many examples where I’ve had to go play hide and seek with data.

In this example I need to extract the end-reason (target) from an assessment (source). For clarity the assessment could look like one of the objects below. I’ve shortened it a bit, for our little demo, we are only interested in two properties claimEndReasonDesc and awardEndReasonTypeDesc.

The problem we are trying to solve is simple, if there is a claimEndReasonDesc then that is the answer, else awardEndReasonTypeDesc, and if neither is there, the answer in an empty string. Sounds really easy? Except our input, the assessment is a bit of an elusive fox. We have to try and find the value we are looking for, cause those properties may not even exist! Better yet, all the properties could be undefined, that’s just the data we have “Life is never easy”.

This is what the data could look like :

const testCaseOne = {claimEndReasonDesc : "The world ended"};
const testCaseTwo = {personRoleServiceAward : [{ awardEndReasonTypeDesc: "The zombie apocalypse"} ]};
const testCaseThree = {};

First up, you might be tempted, like so many developers are, to just start writing if-else-if-else and in this case that is exactly what the previous developer did. Try to think outside the if-else box. What are we trying to do? Here is a clue …. FIND.

The example below (Good Code) just picks the one with a value and returns it. You can think of this as; the first one with a value is the winner. The default at the end is an empty string.

Good Code

I’ve marked this as good code! clean code might be a better description, but I think you can make up your own mind! 😛

If think a better name for this would be the best of 3? Or find the value?

// Version 2.0

export const getEndReasonFrom = (assessment: any): string =>
    assessment?.claimEndReasonDesc ||
    assessment?.personRoleServiceAward?.[0]?.awardEndReasonTypeDesc ||
    "";

const endReason = getEndReasonFrom(assessment);

Is there an abstract concept?

Abstraction is the Dogs Kahunas! right! If we abstract key concepts then we get code reuse, but I can’t see much code re-use going on. Can we use some kind of abstraction instead of our hard coded function, that only works on assessments?

The function above is doing one thing, it’s looking for “the first non undefined value”, and if there is none, then return the default? Is there a function that can do that for us? What if we wrote one? What about this one line function that will work on any array? Do we need to declare a type? Can we use generics? Sure why not.

// Now with generics
export const findFirstNonUndefinedValue = <T>(xs: T[]) => xs.find((x: T) => x !== undefined);

Now that we have our generic find function, that is searching for.. well anything that is NOT undefined, we need a default, what can we do? Just add it to the array. Full example below.

// Version 3.0
export const findFirstNonUndefinedValue = <T>(xs: T[]) => find(xs, (x: T) => 
                x !== undefined);

export const getEndReason = (assessment: any): string =>
  findFirstNonUndefinedValue( [ assessment?.claimEndReasonDesc,
                                assessment?.personRoleServiceAward?.[0]?.awardEndReasonTypeDesc,
                                ""]);

What did we do? We created a function that will accept an array of values, and return the first non-undefined value. I think we acheived that abstract function thing? Will we ever re-use this function? Did we just waste our time on an abstraction with zero re-use. Only time will tell. We might forget about this little abstraction and end up writing something totally different the next time we meet this problem.

It turns out that, the concept above, has repeated itself in our code base… but only twice… :-)

The abstraction allow use to look in more than just two places, that array could be longer. It’s a simple concept, the function tells us what it does. Is it better than version 2.0? For me version 2.0 was just fine, 3.0 take it to another level.

Bad Code - No abstraction

The code below is what most if-else-if-else software engineers would write, some call this just a different style of writing code. It’s not wrong or bad or anything like that. It’s just hard to read, it feels like very little thought has gone into writing it. It works, maybe that’s good enough? Once it’s written it might never be seen again, it might just sit there and do it’s job for 10 years and never be changed. So perhaps it’s just the right level of abstraction?. So what is the problem? Once you start writing code in this style, it can be hard to stop, and the next thing you know you have a 500 line function with 25 nested “if” statements. It’s a gateway to bad code, is what I’m saying!

1. There is a bug with the way optional chaining is used with the array, look closer.
2. Why declare the variable endReason, it’s just not needed.
3. Working out what the value is at the end requires some metal gymnastics.

If you compare it with the code above, it’s just a choice of three possible values, why make it any more difficult than that?

// Version 1.0
 export const getEndReason = (assessment: any) => {
     let endReason = "";
     if (assessment.claimEndReasonDesc) {
         endReason = assessment.claimEndReasonDesc;
     }
     else if (assessment.personRoleServiceAward[0]?.awardEndReasonTypeDesc) {
         endReason = assessment.personRoleServiceAward[0].awardEndReasonTypeDesc;
     }
     return endReason;
 }