Function to determine the direction of an arrow
up vote
1
down vote
favorite
I have a function in a project that I feel could be written better. The purpose of the function is to return the direction of an arrow based on two parameters.
positiveDirection
: The direction for a positive result. The values could beincreasing
ordecreasing
.
changeType
: Whether the result waspositive
,negative
orno-change
.
I guess it's the if
statement that is bothering me the most; it looks like it could be reduced based on some logic gate that I don't know of.
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
javascript ecmascript-6
add a comment |
up vote
1
down vote
favorite
I have a function in a project that I feel could be written better. The purpose of the function is to return the direction of an arrow based on two parameters.
positiveDirection
: The direction for a positive result. The values could beincreasing
ordecreasing
.
changeType
: Whether the result waspositive
,negative
orno-change
.
I guess it's the if
statement that is bothering me the most; it looks like it could be reduced based on some logic gate that I don't know of.
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
javascript ecmascript-6
What's possible values forchangeValue
andpositiveDirection
?
– Calak
Nov 21 at 14:16
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23
add a comment |
up vote
1
down vote
favorite
up vote
1
down vote
favorite
I have a function in a project that I feel could be written better. The purpose of the function is to return the direction of an arrow based on two parameters.
positiveDirection
: The direction for a positive result. The values could beincreasing
ordecreasing
.
changeType
: Whether the result waspositive
,negative
orno-change
.
I guess it's the if
statement that is bothering me the most; it looks like it could be reduced based on some logic gate that I don't know of.
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
javascript ecmascript-6
I have a function in a project that I feel could be written better. The purpose of the function is to return the direction of an arrow based on two parameters.
positiveDirection
: The direction for a positive result. The values could beincreasing
ordecreasing
.
changeType
: Whether the result waspositive
,negative
orno-change
.
I guess it's the if
statement that is bothering me the most; it looks like it could be reduced based on some logic gate that I don't know of.
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
getArrowDirection = (positiveDirection, changeType) => {
let direction = null
if(changeType === 'no-change') {
return direction
}
if(
(changeType === 'positive' && positiveDirection === 'increasing') ||
(changeType === 'negative' && positiveDirection === 'decreasing')
) {
direction = 'up-arrow'
} else if(
(changeType === 'positive' && positiveDirection === 'decreasing') ||
(changeType === 'negative' && positiveDirection === 'increasing')
) {
direction = 'down-arrow'
}
return direction
}
javascript ecmascript-6
javascript ecmascript-6
edited Nov 21 at 16:38
Toby Speight
23.2k538110
23.2k538110
asked Nov 21 at 12:40
Richard Healy
364
364
What's possible values forchangeValue
andpositiveDirection
?
– Calak
Nov 21 at 14:16
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23
add a comment |
What's possible values forchangeValue
andpositiveDirection
?
– Calak
Nov 21 at 14:16
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23
What's possible values for
changeValue
and positiveDirection
?– Calak
Nov 21 at 14:16
What's possible values for
changeValue
and positiveDirection
?– Calak
Nov 21 at 14:16
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23
add a comment |
4 Answers
4
active
oldest
votes
up vote
3
down vote
accepted
If changeValue
can only be (after your early check for 'no-change
') equal to 'positive' or 'negative' and positiveDirection
can only be equal to 'increasing' or 'decreasing, your if
should cover all possible ways.
But you can simplify it a lot making use of the xor operator:
direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')
? 'up-arrow'
: 'down-arrow';
Or make the whole function a one-liner:
return (changeType !== 'no-change')
? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')
: null;
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
GivenA^B
, ifA
andB
are either bothtrue
orfalse
, returnsfalse
. Otherwise, returnstrue
. If fact, you can also replace it with!=
but I found the logic xor prettier/smarter :)
– Calak
Nov 21 at 15:32
add a comment |
up vote
4
down vote
The test for if(changeType === 'no-change') {
is redundant and not needed as it will fall through if the other tests fail and return null
anyways.
You could also break the statement up returning the result as needed and falling through if they fail for null
.
const getArrowDirection = (dir, type) => {
if (type === "positive") {
if (dir === "increasing") { return "up-arrow" }
if (dir === "decreasing") { return "down-arrow" }
}else if (type === "negative") {
if (dir === "increasing") { return "down-arrow" }
if (dir === "decreasing") { return "up-arrow" }
}
return null;
}
Assuming that you have given all possible values you can make assumptions and reduce the code further
const getArrowDirection = (dir, type) => {
if (type !== 'no-change') {
if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }
return dir === "increasing" ? "down-arrow" : "up-arrow";
}
return null; // returning null is not the best. Returning undefined would
// be better and would not need this line
}
You can use an object as a lookup using the combined strings.
const getArrowDirection = (() => {
const directions = {
positiveincreasing: "up-arrow",
negativedecreasing: "up-arrow",
positivedecreasing: "down-arrow",
negativeincreasing: "down-arrow",
};
return (dir, type) => directions[type + dir] ? directions[type + dir] : null;
})();
add a comment |
up vote
2
down vote
We don't need the direction
variable - we can simply return at the appropriate point.
Once you know that changeType
is one of 'positive'
or 'negative'
and that positiveDirection
is either 'increasing'
or 'decreasing'
, you can test whether the two equality tests match:
// Winging it, because this isn't my language!
getArrowDirection = (positiveDirection, changeType) => {
if (changeType != 'positive' && changeType != 'negative') {
return null;
}
if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')
return null;
}
if ((changeType === 'positive') == (positiveDirection === 'increasing')) {
return 'up-arrow';
else
return 'down-arrow';
}
}
add a comment |
up vote
0
down vote
You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.
add a comment |
4 Answers
4
active
oldest
votes
4 Answers
4
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
3
down vote
accepted
If changeValue
can only be (after your early check for 'no-change
') equal to 'positive' or 'negative' and positiveDirection
can only be equal to 'increasing' or 'decreasing, your if
should cover all possible ways.
But you can simplify it a lot making use of the xor operator:
direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')
? 'up-arrow'
: 'down-arrow';
Or make the whole function a one-liner:
return (changeType !== 'no-change')
? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')
: null;
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
GivenA^B
, ifA
andB
are either bothtrue
orfalse
, returnsfalse
. Otherwise, returnstrue
. If fact, you can also replace it with!=
but I found the logic xor prettier/smarter :)
– Calak
Nov 21 at 15:32
add a comment |
up vote
3
down vote
accepted
If changeValue
can only be (after your early check for 'no-change
') equal to 'positive' or 'negative' and positiveDirection
can only be equal to 'increasing' or 'decreasing, your if
should cover all possible ways.
But you can simplify it a lot making use of the xor operator:
direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')
? 'up-arrow'
: 'down-arrow';
Or make the whole function a one-liner:
return (changeType !== 'no-change')
? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')
: null;
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
GivenA^B
, ifA
andB
are either bothtrue
orfalse
, returnsfalse
. Otherwise, returnstrue
. If fact, you can also replace it with!=
but I found the logic xor prettier/smarter :)
– Calak
Nov 21 at 15:32
add a comment |
up vote
3
down vote
accepted
up vote
3
down vote
accepted
If changeValue
can only be (after your early check for 'no-change
') equal to 'positive' or 'negative' and positiveDirection
can only be equal to 'increasing' or 'decreasing, your if
should cover all possible ways.
But you can simplify it a lot making use of the xor operator:
direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')
? 'up-arrow'
: 'down-arrow';
Or make the whole function a one-liner:
return (changeType !== 'no-change')
? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')
: null;
If changeValue
can only be (after your early check for 'no-change
') equal to 'positive' or 'negative' and positiveDirection
can only be equal to 'increasing' or 'decreasing, your if
should cover all possible ways.
But you can simplify it a lot making use of the xor operator:
direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')
? 'up-arrow'
: 'down-arrow';
Or make the whole function a one-liner:
return (changeType !== 'no-change')
? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')
: null;
answered Nov 21 at 14:43
Calak
2,102318
2,102318
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
GivenA^B
, ifA
andB
are either bothtrue
orfalse
, returnsfalse
. Otherwise, returnstrue
. If fact, you can also replace it with!=
but I found the logic xor prettier/smarter :)
– Calak
Nov 21 at 15:32
add a comment |
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
GivenA^B
, ifA
andB
are either bothtrue
orfalse
, returnsfalse
. Otherwise, returnstrue
. If fact, you can also replace it with!=
but I found the logic xor prettier/smarter :)
– Calak
Nov 21 at 15:32
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
This is slick. Definitely a better solution than my original code. The ^ syntax was something I didn't know about.
– Richard Healy
Nov 21 at 15:21
Given
A^B
, if A
and B
are either both true
or false
, returns false
. Otherwise, returns true
. If fact, you can also replace it with !=
but I found the logic xor prettier/smarter :)– Calak
Nov 21 at 15:32
Given
A^B
, if A
and B
are either both true
or false
, returns false
. Otherwise, returns true
. If fact, you can also replace it with !=
but I found the logic xor prettier/smarter :)– Calak
Nov 21 at 15:32
add a comment |
up vote
4
down vote
The test for if(changeType === 'no-change') {
is redundant and not needed as it will fall through if the other tests fail and return null
anyways.
You could also break the statement up returning the result as needed and falling through if they fail for null
.
const getArrowDirection = (dir, type) => {
if (type === "positive") {
if (dir === "increasing") { return "up-arrow" }
if (dir === "decreasing") { return "down-arrow" }
}else if (type === "negative") {
if (dir === "increasing") { return "down-arrow" }
if (dir === "decreasing") { return "up-arrow" }
}
return null;
}
Assuming that you have given all possible values you can make assumptions and reduce the code further
const getArrowDirection = (dir, type) => {
if (type !== 'no-change') {
if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }
return dir === "increasing" ? "down-arrow" : "up-arrow";
}
return null; // returning null is not the best. Returning undefined would
// be better and would not need this line
}
You can use an object as a lookup using the combined strings.
const getArrowDirection = (() => {
const directions = {
positiveincreasing: "up-arrow",
negativedecreasing: "up-arrow",
positivedecreasing: "down-arrow",
negativeincreasing: "down-arrow",
};
return (dir, type) => directions[type + dir] ? directions[type + dir] : null;
})();
add a comment |
up vote
4
down vote
The test for if(changeType === 'no-change') {
is redundant and not needed as it will fall through if the other tests fail and return null
anyways.
You could also break the statement up returning the result as needed and falling through if they fail for null
.
const getArrowDirection = (dir, type) => {
if (type === "positive") {
if (dir === "increasing") { return "up-arrow" }
if (dir === "decreasing") { return "down-arrow" }
}else if (type === "negative") {
if (dir === "increasing") { return "down-arrow" }
if (dir === "decreasing") { return "up-arrow" }
}
return null;
}
Assuming that you have given all possible values you can make assumptions and reduce the code further
const getArrowDirection = (dir, type) => {
if (type !== 'no-change') {
if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }
return dir === "increasing" ? "down-arrow" : "up-arrow";
}
return null; // returning null is not the best. Returning undefined would
// be better and would not need this line
}
You can use an object as a lookup using the combined strings.
const getArrowDirection = (() => {
const directions = {
positiveincreasing: "up-arrow",
negativedecreasing: "up-arrow",
positivedecreasing: "down-arrow",
negativeincreasing: "down-arrow",
};
return (dir, type) => directions[type + dir] ? directions[type + dir] : null;
})();
add a comment |
up vote
4
down vote
up vote
4
down vote
The test for if(changeType === 'no-change') {
is redundant and not needed as it will fall through if the other tests fail and return null
anyways.
You could also break the statement up returning the result as needed and falling through if they fail for null
.
const getArrowDirection = (dir, type) => {
if (type === "positive") {
if (dir === "increasing") { return "up-arrow" }
if (dir === "decreasing") { return "down-arrow" }
}else if (type === "negative") {
if (dir === "increasing") { return "down-arrow" }
if (dir === "decreasing") { return "up-arrow" }
}
return null;
}
Assuming that you have given all possible values you can make assumptions and reduce the code further
const getArrowDirection = (dir, type) => {
if (type !== 'no-change') {
if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }
return dir === "increasing" ? "down-arrow" : "up-arrow";
}
return null; // returning null is not the best. Returning undefined would
// be better and would not need this line
}
You can use an object as a lookup using the combined strings.
const getArrowDirection = (() => {
const directions = {
positiveincreasing: "up-arrow",
negativedecreasing: "up-arrow",
positivedecreasing: "down-arrow",
negativeincreasing: "down-arrow",
};
return (dir, type) => directions[type + dir] ? directions[type + dir] : null;
})();
The test for if(changeType === 'no-change') {
is redundant and not needed as it will fall through if the other tests fail and return null
anyways.
You could also break the statement up returning the result as needed and falling through if they fail for null
.
const getArrowDirection = (dir, type) => {
if (type === "positive") {
if (dir === "increasing") { return "up-arrow" }
if (dir === "decreasing") { return "down-arrow" }
}else if (type === "negative") {
if (dir === "increasing") { return "down-arrow" }
if (dir === "decreasing") { return "up-arrow" }
}
return null;
}
Assuming that you have given all possible values you can make assumptions and reduce the code further
const getArrowDirection = (dir, type) => {
if (type !== 'no-change') {
if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }
return dir === "increasing" ? "down-arrow" : "up-arrow";
}
return null; // returning null is not the best. Returning undefined would
// be better and would not need this line
}
You can use an object as a lookup using the combined strings.
const getArrowDirection = (() => {
const directions = {
positiveincreasing: "up-arrow",
negativedecreasing: "up-arrow",
positivedecreasing: "down-arrow",
negativeincreasing: "down-arrow",
};
return (dir, type) => directions[type + dir] ? directions[type + dir] : null;
})();
edited Nov 21 at 15:18
answered Nov 21 at 13:11
Blindman67
6,7541521
6,7541521
add a comment |
add a comment |
up vote
2
down vote
We don't need the direction
variable - we can simply return at the appropriate point.
Once you know that changeType
is one of 'positive'
or 'negative'
and that positiveDirection
is either 'increasing'
or 'decreasing'
, you can test whether the two equality tests match:
// Winging it, because this isn't my language!
getArrowDirection = (positiveDirection, changeType) => {
if (changeType != 'positive' && changeType != 'negative') {
return null;
}
if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')
return null;
}
if ((changeType === 'positive') == (positiveDirection === 'increasing')) {
return 'up-arrow';
else
return 'down-arrow';
}
}
add a comment |
up vote
2
down vote
We don't need the direction
variable - we can simply return at the appropriate point.
Once you know that changeType
is one of 'positive'
or 'negative'
and that positiveDirection
is either 'increasing'
or 'decreasing'
, you can test whether the two equality tests match:
// Winging it, because this isn't my language!
getArrowDirection = (positiveDirection, changeType) => {
if (changeType != 'positive' && changeType != 'negative') {
return null;
}
if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')
return null;
}
if ((changeType === 'positive') == (positiveDirection === 'increasing')) {
return 'up-arrow';
else
return 'down-arrow';
}
}
add a comment |
up vote
2
down vote
up vote
2
down vote
We don't need the direction
variable - we can simply return at the appropriate point.
Once you know that changeType
is one of 'positive'
or 'negative'
and that positiveDirection
is either 'increasing'
or 'decreasing'
, you can test whether the two equality tests match:
// Winging it, because this isn't my language!
getArrowDirection = (positiveDirection, changeType) => {
if (changeType != 'positive' && changeType != 'negative') {
return null;
}
if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')
return null;
}
if ((changeType === 'positive') == (positiveDirection === 'increasing')) {
return 'up-arrow';
else
return 'down-arrow';
}
}
We don't need the direction
variable - we can simply return at the appropriate point.
Once you know that changeType
is one of 'positive'
or 'negative'
and that positiveDirection
is either 'increasing'
or 'decreasing'
, you can test whether the two equality tests match:
// Winging it, because this isn't my language!
getArrowDirection = (positiveDirection, changeType) => {
if (changeType != 'positive' && changeType != 'negative') {
return null;
}
if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')
return null;
}
if ((changeType === 'positive') == (positiveDirection === 'increasing')) {
return 'up-arrow';
else
return 'down-arrow';
}
}
answered Nov 21 at 14:17
Toby Speight
23.2k538110
23.2k538110
add a comment |
add a comment |
up vote
0
down vote
You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.
add a comment |
up vote
0
down vote
You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.
add a comment |
up vote
0
down vote
up vote
0
down vote
You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.
You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.
answered Nov 21 at 13:09
Dave
1
1
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208133%2ffunction-to-determine-the-direction-of-an-arrow%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
What's possible values for
changeValue
andpositiveDirection
?– Calak
Nov 21 at 14:16
We might be able to improve the code further if we knew the context. Who calls this code, and where do the parameter values come from?
– 200_success
Nov 21 at 15:23