Reputation: 41
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!
Upvotes: 0
Views: 107
Reputation:
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
};
Upvotes: 0
Reputation: 27192
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
}
Upvotes: 0
Reputation: 38094
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)
Upvotes: 1
Reputation: 545
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
}
Upvotes: 1
Reputation: 3635
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 !
Upvotes: 4
Reputation: 402
Without questioning the premise of the question, you can write :
const isEmpty():boolean => {
return hasValArr1() || hasValArr2()
}
Upvotes: 1