What's an explicit way that I can be sure that the only results for a boolean check will be "true" or "false"? In other words, I want to exclude "undefined" as a possibility as much as is possible. Two options would be:
FUNCTION ONE:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
if (service.currentStage === 'enrolling') {
return true;
}
}
}
}
}
FUNCTION TWO:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
return service.currentStage === 'enrolling';
}
}
}
}
EDIT: Upon a menter's response, a more robust alternative is to explicitly return 'false', like this:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
//You should also rethink this return statement
return service.currentStage === 'enrolling';
}
}
}
return false;
}
I would follow that up by asking if adding an extra "return 'false'" for a case where "client.services" exists, but "currentStage !== 'enrolling" would be even better? Or would that second 'else' clause be redundant in this case?
Secondly, he writes I should rethink the return statement in terms of where it is in the function. What would the alternative be? In short, I'm trying to find the most robust yet terse way to write this function.
What's an explicit way that I can be sure that the only results for a boolean check will be "true" or "false"? In other words, I want to exclude "undefined" as a possibility as much as is possible. Two options would be:
FUNCTION ONE:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
if (service.currentStage === 'enrolling') {
return true;
}
}
}
}
}
FUNCTION TWO:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
return service.currentStage === 'enrolling';
}
}
}
}
EDIT: Upon a menter's response, a more robust alternative is to explicitly return 'false', like this:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
//You should also rethink this return statement
return service.currentStage === 'enrolling';
}
}
}
return false;
}
I would follow that up by asking if adding an extra "return 'false'" for a case where "client.services" exists, but "currentStage !== 'enrolling" would be even better? Or would that second 'else' clause be redundant in this case?
Secondly, he writes I should rethink the return statement in terms of where it is in the function. What would the alternative be? In short, I'm trying to find the most robust yet terse way to write this function.
Share Improve this question edited Jun 29, 2017 at 23:26 Rey asked Jun 29, 2017 at 22:32 ReyRey 1,4332 gold badges17 silver badges32 bronze badges 6- Possible duplicate of Are these JS conditional statements functionally equivalent? – PM 77-1 Commented Jun 29, 2017 at 22:35
-
The first will return
true
orundefined
, the secondtrue
,false
orundefined
. – RobG Commented Jun 29, 2017 at 22:36 -
The first will return a value as soon as
service === currentOptionSelected
istrue
, while the second will continue to iterate over the remaining elements if the innerif
statement isfalse
. – castletheperson Commented Jun 29, 2017 at 22:36 - I'm asking a more nuanced question here. In the previous question I did NOT include the second of these options. So it's not the same question. I'm specifically trying to clarify when "false" is returned as the only alternative to "true", rather than getting "true", "false", or "undefined". In other words, I want to avoid "undefined" as a possibility. – Rey Commented Jun 29, 2017 at 22:40
- All these questions have been already answered. – PM 77-1 Commented Jun 29, 2017 at 22:49
2 Answers
Reset to default 2These are not equivalent. The second version returns false
in some of the cases where the first one returns undefined
. A caller that checks the value to be explicitly === false
will observe different behavior.
Both can return undefined
which is probably not great. It'd be best to always return with an actual value of true
or false
Short answer is No, they aren't equivalent:
- The first one won't return
false
in any case. - While the second can return
false
ifservice.currentStage !== 'enrolling'
.
But as stated by Ryan both can return undefined
which you should avoid, you need to explicitly return false
whenever a condition isn't met.
This is how should be your code:
private canMove = (currentOptionSelected): boolean => {
if (this.client.services) {
for (const service of this.client.services) {
if (service === currentOptionSelected) {
//You should also rethink this return statement
return service.currentStage === 'enrolling';
}
}
}
return false;
}
Note:
- The
return false;
here will make sure you returnfalse
whenthis.client.services
isundefined
. - Using a
return
statement in afor
loop this way is a very very bad idea, in fact you will only make one iteration.