I am working on a project and in the code I added a few if statements. I was then told that I can do it on line and more efficiently. The way I did works perfectly but I need to refactor to get it accepted. Could you please help me out? I have tried ternary operator as you can see in the examples below but it's still not that short

Assuming that we have two arrays arr1 and arr2 and the following code is implemented to check if their lengths.

const hasValArr1 = ():boolean => return arr1.length > 0
const hasValArr2 = ():boolean => return arr2.length > 0

Now the interesting part if statements

const isEmpty():boolean => {

if (!hasValArr1() && !hasValArr2()) return false

else if (hasValArr1() && hasValArr2()) return true 

else if (!hasValArr1() && hasValArr2()) return true 

else if (hasValArr1() && !hasValArr2()) return true

}

using ternary operator

 (!hasValArr1() && !hasValArr2()) ? false

:(hasValArr1() && hasValArr2()) ? true 

:(!hasValArr1() && hasValArr2()) ? true 

:(hasValArr1() && !hasValArr2()) && true

How would you go to write this in a more readable and efficient way? Thanks in advance!

CodePudding user response：

The implementation does not match the name of the method. The name of the method is isEmpty but it returns false if both arrays don't have a value: if (!hasValArr1() && !hasValArr2()) return false

So the name should be: hasAnyValue or doArraysHaveAnyValue or something of that sorts.

As for simplification, you can simply use ||:

const doArraysHaveAnyValue(): boolean => {
    return hasValArr1() || hasValArr2();
}

The reason this is better is that it is easier to read, and gives preference to using "positive" instead of negation with !

CodePudding user response：

No need to create separate function for check array length, you can directly use in isEmpty() function and get boolean value

const isEmpty():boolean => {
    return arr1.length > 0 || arr2.length > 0
}

CodePudding user response：

Without questioning the premise of the question, you can write :

const isEmpty():boolean => {
   return hasValArr1() || hasValArr2()
}

CodePudding user response：

I think you can

сonst isEmpty():boolean => {

    if (!hasValArr1() && !hasValArr2()) 
       return false
    
       return true;
}

or:

const isEmpty():boolean => {
   return (!hasValArr1() && !hasValArr2())
}

or if you want to check whether the both arrays have values:

const HasArraysData():boolean => {
   return (hasValArr1() && hasValArr2())
}

and it becomes simpler to read code:

if (HasArraysData)

or:

if (!HasArraysData)

CodePudding user response：

So as you write, only the first case you want to return false , other cases you want to return only true , that is why your codes will be like below

const isEmpty():boolean => {

if (!hasValArr1() && !hasValArr2()) return false

else return true

};

CodePudding user response：

If you want to proceed with the approach you mentioned, Then there are few suggestions or nice to have :

• Instead of return arr1.length > 0 you can simply use return arr1.length

• No need to use multiple conditions as you are checking if both the arrays contains value or not. Hence, you can simply modify your isEmpty function like this :

  const isEmpty() : boolean => {
    return (!hasValArr1() && !hasValArr2())
  }
  

My suggestion is instead of checking arr1 and arr2 length in the separate methods. You can check that in isEmpty() method itself.

const isEmpty() : boolean => {
    return !arr1.length && !arr2.length
}

CodePudding user response：

const isEmpty():boolean => {
    return hasValArr1() || hasValArr2()
}