Simplify an if/else statement? [on hold]
up vote
14
down vote
favorite
I'm trying to simplify the following:
function handleDirection(src) {
if (src === 'left') {
if (inverse) {
tracker--;
} else {
tracker++;
}
} else {
if (inverse) {
tracker++;
} else {
tracker--;
}
}
}
to reduce the number of conditionals. The src
will either be 'left'
or 'right'
always.
javascript if-statement conditional
put on hold as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed 9 hours ago
- This question does not appear to be about programming within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.
|
show 6 more comments
up vote
14
down vote
favorite
I'm trying to simplify the following:
function handleDirection(src) {
if (src === 'left') {
if (inverse) {
tracker--;
} else {
tracker++;
}
} else {
if (inverse) {
tracker++;
} else {
tracker--;
}
}
}
to reduce the number of conditionals. The src
will either be 'left'
or 'right'
always.
javascript if-statement conditional
put on hold as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed 9 hours ago
- This question does not appear to be about programming within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.
8
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
12
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
8
Side note: your function uses 3 variables (src
,inverse
andtracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure theif
s....
– Peter B
17 hours ago
1
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.
– vlaz
17 hours ago
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago
|
show 6 more comments
up vote
14
down vote
favorite
up vote
14
down vote
favorite
I'm trying to simplify the following:
function handleDirection(src) {
if (src === 'left') {
if (inverse) {
tracker--;
} else {
tracker++;
}
} else {
if (inverse) {
tracker++;
} else {
tracker--;
}
}
}
to reduce the number of conditionals. The src
will either be 'left'
or 'right'
always.
javascript if-statement conditional
I'm trying to simplify the following:
function handleDirection(src) {
if (src === 'left') {
if (inverse) {
tracker--;
} else {
tracker++;
}
} else {
if (inverse) {
tracker++;
} else {
tracker--;
}
}
}
to reduce the number of conditionals. The src
will either be 'left'
or 'right'
always.
javascript if-statement conditional
javascript if-statement conditional
edited 10 hours ago
Charlie Harding
261414
261414
asked 17 hours ago
Rebecca O'Sullivan
373114
373114
put on hold as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed 9 hours ago
- This question does not appear to be about programming within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.
put on hold as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed 9 hours ago
- This question does not appear to be about programming within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.
8
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
12
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
8
Side note: your function uses 3 variables (src
,inverse
andtracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure theif
s....
– Peter B
17 hours ago
1
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.
– vlaz
17 hours ago
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago
|
show 6 more comments
8
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
12
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
8
Side note: your function uses 3 variables (src
,inverse
andtracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure theif
s....
– Peter B
17 hours ago
1
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.
– vlaz
17 hours ago
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago
8
8
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
12
12
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
8
8
Side note: your function uses 3 variables (
src
, inverse
and tracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure the if
s....– Peter B
17 hours ago
Side note: your function uses 3 variables (
src
, inverse
and tracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure the if
s....– Peter B
17 hours ago
1
1
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (
tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.– vlaz
17 hours ago
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (
tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.– vlaz
17 hours ago
3
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago
|
show 6 more comments
12 Answers
12
active
oldest
votes
up vote
31
down vote
accepted
You could check with the result of the first check.
This is an exclusive OR check.
// typeof inverse === 'boolean'
function handleDirection(src) {
if ((src === 'left') === inverse) {
tracker--;
} else {
tracker++;
}
}
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
^^ only in this special case
– Nina Scholz
17 hours ago
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
|
show 7 more comments
up vote
9
down vote
function handleDirection(src) {
var i = 1;
if(src === 'left')
i = -1;
if(inverse)
tracker += i;
else
tracker -= i;
}
2
This creates an unnecessary extra variable imo... Usingtracker--
andtracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.
– MagicLegend
17 hours ago
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name likeadjustment
ormovement
.
– TheRubberDuck
11 hours ago
add a comment |
up vote
8
down vote
You can even do it with just one line of Code:
function getDirectionOffset(src) {
tracker += (src === 'left' ? 1 : -1) * (inverse ? -1 : 1);
}
New contributor
add a comment |
up vote
5
down vote
This could be simplified to a ternary expression which returns 1
or -1
depending on the state. Then you can just add that to the tracker
.
function handleDirection(src) {
var delta = (src === 'left' && inverse) || (src !== 'left' && !inverse) ? -1 : 1;
tracker += delta;
}
This could then be simplified further using the logic which @NinaScholz pointed out in her answer:
function handleDirection(src) {
var delta = (src === 'left') === inverse ? -1 : 1;
tracker += delta;
}
add a comment |
up vote
4
down vote
You want to increase the tracker if one of src == left
or inverse
is true but not the other, and decrease it otherwise, which is what the "XOR" ^
operator does :
function handleDirection(src) {
if (src === 'left' ^ inverse) {
tracker++;
} else {
tracker--;
}
}
You can reduce that further by using a ternary expression :
function handleDirection(src) {
tracker += src === 'left' ^ inverse ? 1 : -1;
}
Or if you want to avoid any kind of conditionnal, with implicit casts and "clever" arithmetics :
function handleDirection(src) {
tracker += 1 - 2 * (src === 'right' ^ inverse);
}
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
add a comment |
up vote
3
down vote
Assuming inverse
is a flag you'd set once, then you don't need to take it into account every time, you can calculate its impact once and just use it as it is, which will cut down your code branches and logic. If you want to change it as you go along, then you might need to separate the logic for the calculation, in order to re-use it.
You can also then extract the movement direction into a self-contained function and your handleDirection
becomes very simple - you calculate the direction you want to go based on src
and the invert
.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
With that said, all this suggests you shouldn't be using a function, or you should be using it differently. You can collect all that functionality in a class or instead have all the information passed around functions, so you don't have globals. Here is a sample object oriented implementation of the concept:
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
By the way, another alternative is to NOT compute the movement every time. You actually know what is happening every time - in effect, you have a truth table where your inputs are src === left
and inverse
and the outputs are how you modify your tracking.
+--------+------------+--------+
| isLeft | isInverted | Offset |
+--------+------------+--------+
| true | true | -1 |
| true | false | 1 |
| false | true | 1 |
| false | false | -1 |
+--------+------------+--------+
So, you can just put that table in.
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
In this case it might be an overkill but this approach might be useful if there are more flags (including more values for the flags) and/or end states. For example, maybe you want to introduce four directions, but you don't modify the tracker
value if it's up
or down
.
+-----------+------------+--------+
| direction | isInverted | Offset |
+-----------+------------+--------+
| left | true | -1 |
| left | false | 1 |
| right | true | 1 |
| right | false | -1 |
| up | false | 0 |
| up | true | 0 |
| down | false | 0 |
| down | true | 0 |
+-----------+------------+--------+
As you can see, now it's not just booleans, you can handle any value. Using a table, you also then change invert
to be something like windDirection
, so if the movement is left
and the windDirection
is right
, the result is like what it is now, but you could have direction of left
and wind going left
, so you move further. Or you can move up
and the wind direction is left
so tracker
(at this point the X coordinates) is going to actually be modified.
+-----------+---------------+---------+
| direction | windDirection | OffsetX |
+-----------+---------------+---------+
| left | right | -1 |
| left | up | 1 |
| left | down | 1 |
| left | left | 2 |
| right | up | -1 |
| right | down | -1 |
| right | right | -2 |
| right | left | 1 |
| up | up | 0 |
| up | down | 0 |
| up | left | 1 |
| up | right | -1 |
| down | up | 0 |
| down | down | 0 |
| down | left | 1 |
| down | right | -1 |
+-----------+---------------+---------+
With four directions and four wind directions to take into account the logic can be quite annoying to both read and maintain in the future, while if you only have a lookup table, it's easy and you can easily extend this to even handle diagonals (let's assume they change the value by 0.5
instead of 1
) and your algorithm would not really care as long as you just fetch the values from the table.
add a comment |
up vote
1
down vote
Right now you are comparing on strings, which I wouldn't advise. If for example you use 'Left' instead of 'left' it will fail the first if statement. Perhaps a boolean could be of use here, since you can guarantee it only has two states.
The if statements inside can be compressed via conditional operators.
Perhaps something like this is what you are looking for:
function handleDirection(src) {
if (src) {
inverse ? tracker-- : tracker++;
} else {
inverse ? tracker-- : tracker++;
}
}
See: https://jsfiddle.net/9zr4f3nv/
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
add a comment |
up vote
1
down vote
You can use short circuiting syntax or ternary operators
// by using short circuiting
function handleDirection(src) {
if (src == 'left') tracker = inverse && tracker-1 || tracker +1
else tracker = inverse && tracker+1 || tracker -1
}
// by using ternary operator
function handleDirection(src) {
if (src == 'left') tracker = inverse ? tracker-1 : tracker +1
else tracker = inverse ? tracker+1 : tracker -1
}
add a comment |
up vote
1
down vote
Not the shortest possible, but hopefully clear:
const LEFT = 'left'
const RIGHT = 'right'
function handleMovement(src) {
tracker += movementIncrement(src, inverse)
}
function movementIncrement(direction, inverse) {
if (inverse) {
direction = inverseDirection(direction)
}
return LEFT ? -1 : +1
}
function inverseDirection(direction) {
return direction === LEFT ? RIGHT : LEFT
}
add a comment |
up vote
1
down vote
This has only one conditional, and I find it reads more intuitively than the other answers:
function handleDirection(src) {
if (
((src === 'left') && !inverse) ||
((src === 'right') && inverse)
) {
tracker++;
}
else {
tracker--;
}
}
add a comment |
up vote
1
down vote
I dislike elses and try to avoid nesting if possible. I think this conveys the idea of inverse
in a more natural way:
function handleDirection(src)
{
let change = 1;
if ('right' == src)
change = -1;
if (inverse)
change = -change;
tracker += change;
}
add a comment |
up vote
-1
down vote
You could use an 2 dimensional array type data structure from js and store the desired outcomes at index sec and inverse. Or JSON.
add a comment |
12 Answers
12
active
oldest
votes
12 Answers
12
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
31
down vote
accepted
You could check with the result of the first check.
This is an exclusive OR check.
// typeof inverse === 'boolean'
function handleDirection(src) {
if ((src === 'left') === inverse) {
tracker--;
} else {
tracker++;
}
}
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
^^ only in this special case
– Nina Scholz
17 hours ago
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
|
show 7 more comments
up vote
31
down vote
accepted
You could check with the result of the first check.
This is an exclusive OR check.
// typeof inverse === 'boolean'
function handleDirection(src) {
if ((src === 'left') === inverse) {
tracker--;
} else {
tracker++;
}
}
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
^^ only in this special case
– Nina Scholz
17 hours ago
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
|
show 7 more comments
up vote
31
down vote
accepted
up vote
31
down vote
accepted
You could check with the result of the first check.
This is an exclusive OR check.
// typeof inverse === 'boolean'
function handleDirection(src) {
if ((src === 'left') === inverse) {
tracker--;
} else {
tracker++;
}
}
You could check with the result of the first check.
This is an exclusive OR check.
// typeof inverse === 'boolean'
function handleDirection(src) {
if ((src === 'left') === inverse) {
tracker--;
} else {
tracker++;
}
}
edited 10 hours ago
wizzwizz4
3,23811334
3,23811334
answered 17 hours ago
Nina Scholz
173k1387149
173k1387149
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
^^ only in this special case
– Nina Scholz
17 hours ago
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
|
show 7 more comments
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
^^ only in this special case
– Nina Scholz
17 hours ago
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
this is a good answer - was unaware you could chain conditions like this .. !!
– treyBake
17 hours ago
2
2
^^ only in this special case
– Nina Scholz
17 hours ago
^^ only in this special case
– Nina Scholz
17 hours ago
1
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
17 hours ago
6
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
11 hours ago
5
5
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
I'd use this only if you really need the performance boost and/or only work with people who'd be able to intuitively read this and come up with this. The other approach maybe longer, but it's much more readable to the average developer.
– Darkwing
11 hours ago
|
show 7 more comments
up vote
9
down vote
function handleDirection(src) {
var i = 1;
if(src === 'left')
i = -1;
if(inverse)
tracker += i;
else
tracker -= i;
}
2
This creates an unnecessary extra variable imo... Usingtracker--
andtracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.
– MagicLegend
17 hours ago
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name likeadjustment
ormovement
.
– TheRubberDuck
11 hours ago
add a comment |
up vote
9
down vote
function handleDirection(src) {
var i = 1;
if(src === 'left')
i = -1;
if(inverse)
tracker += i;
else
tracker -= i;
}
2
This creates an unnecessary extra variable imo... Usingtracker--
andtracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.
– MagicLegend
17 hours ago
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name likeadjustment
ormovement
.
– TheRubberDuck
11 hours ago
add a comment |
up vote
9
down vote
up vote
9
down vote
function handleDirection(src) {
var i = 1;
if(src === 'left')
i = -1;
if(inverse)
tracker += i;
else
tracker -= i;
}
function handleDirection(src) {
var i = 1;
if(src === 'left')
i = -1;
if(inverse)
tracker += i;
else
tracker -= i;
}
answered 17 hours ago
Alays
1748
1748
2
This creates an unnecessary extra variable imo... Usingtracker--
andtracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.
– MagicLegend
17 hours ago
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name likeadjustment
ormovement
.
– TheRubberDuck
11 hours ago
add a comment |
2
This creates an unnecessary extra variable imo... Usingtracker--
andtracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.
– MagicLegend
17 hours ago
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name likeadjustment
ormovement
.
– TheRubberDuck
11 hours ago
2
2
This creates an unnecessary extra variable imo... Using
tracker--
and tracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.– MagicLegend
17 hours ago
This creates an unnecessary extra variable imo... Using
tracker--
and tracker++
is the correct way to increase and decrease the variable in this case. If it were desired to increase or lower the variable with more than one this might be a good guideline.– MagicLegend
17 hours ago
2
2
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name like
adjustment
or movement
.– TheRubberDuck
11 hours ago
@MagicLegend Actually, I think the variable helps bring "real world" parity to the solution. The other answers focus on "efficiency" which is probably irrelevant in such a simple case. The interpreter doesn't need help reading, but humans do. Although I upvoted, I would go further and give the variable a more meaningful name like
adjustment
or movement
.– TheRubberDuck
11 hours ago
add a comment |
up vote
8
down vote
You can even do it with just one line of Code:
function getDirectionOffset(src) {
tracker += (src === 'left' ? 1 : -1) * (inverse ? -1 : 1);
}
New contributor
add a comment |
up vote
8
down vote
You can even do it with just one line of Code:
function getDirectionOffset(src) {
tracker += (src === 'left' ? 1 : -1) * (inverse ? -1 : 1);
}
New contributor
add a comment |
up vote
8
down vote
up vote
8
down vote
You can even do it with just one line of Code:
function getDirectionOffset(src) {
tracker += (src === 'left' ? 1 : -1) * (inverse ? -1 : 1);
}
New contributor
You can even do it with just one line of Code:
function getDirectionOffset(src) {
tracker += (src === 'left' ? 1 : -1) * (inverse ? -1 : 1);
}
New contributor
New contributor
answered 15 hours ago
Leuronics
811
811
New contributor
New contributor
add a comment |
add a comment |
up vote
5
down vote
This could be simplified to a ternary expression which returns 1
or -1
depending on the state. Then you can just add that to the tracker
.
function handleDirection(src) {
var delta = (src === 'left' && inverse) || (src !== 'left' && !inverse) ? -1 : 1;
tracker += delta;
}
This could then be simplified further using the logic which @NinaScholz pointed out in her answer:
function handleDirection(src) {
var delta = (src === 'left') === inverse ? -1 : 1;
tracker += delta;
}
add a comment |
up vote
5
down vote
This could be simplified to a ternary expression which returns 1
or -1
depending on the state. Then you can just add that to the tracker
.
function handleDirection(src) {
var delta = (src === 'left' && inverse) || (src !== 'left' && !inverse) ? -1 : 1;
tracker += delta;
}
This could then be simplified further using the logic which @NinaScholz pointed out in her answer:
function handleDirection(src) {
var delta = (src === 'left') === inverse ? -1 : 1;
tracker += delta;
}
add a comment |
up vote
5
down vote
up vote
5
down vote
This could be simplified to a ternary expression which returns 1
or -1
depending on the state. Then you can just add that to the tracker
.
function handleDirection(src) {
var delta = (src === 'left' && inverse) || (src !== 'left' && !inverse) ? -1 : 1;
tracker += delta;
}
This could then be simplified further using the logic which @NinaScholz pointed out in her answer:
function handleDirection(src) {
var delta = (src === 'left') === inverse ? -1 : 1;
tracker += delta;
}
This could be simplified to a ternary expression which returns 1
or -1
depending on the state. Then you can just add that to the tracker
.
function handleDirection(src) {
var delta = (src === 'left' && inverse) || (src !== 'left' && !inverse) ? -1 : 1;
tracker += delta;
}
This could then be simplified further using the logic which @NinaScholz pointed out in her answer:
function handleDirection(src) {
var delta = (src === 'left') === inverse ? -1 : 1;
tracker += delta;
}
edited 17 hours ago
answered 17 hours ago
Rory McCrossan
240k29203244
240k29203244
add a comment |
add a comment |
up vote
4
down vote
You want to increase the tracker if one of src == left
or inverse
is true but not the other, and decrease it otherwise, which is what the "XOR" ^
operator does :
function handleDirection(src) {
if (src === 'left' ^ inverse) {
tracker++;
} else {
tracker--;
}
}
You can reduce that further by using a ternary expression :
function handleDirection(src) {
tracker += src === 'left' ^ inverse ? 1 : -1;
}
Or if you want to avoid any kind of conditionnal, with implicit casts and "clever" arithmetics :
function handleDirection(src) {
tracker += 1 - 2 * (src === 'right' ^ inverse);
}
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
add a comment |
up vote
4
down vote
You want to increase the tracker if one of src == left
or inverse
is true but not the other, and decrease it otherwise, which is what the "XOR" ^
operator does :
function handleDirection(src) {
if (src === 'left' ^ inverse) {
tracker++;
} else {
tracker--;
}
}
You can reduce that further by using a ternary expression :
function handleDirection(src) {
tracker += src === 'left' ^ inverse ? 1 : -1;
}
Or if you want to avoid any kind of conditionnal, with implicit casts and "clever" arithmetics :
function handleDirection(src) {
tracker += 1 - 2 * (src === 'right' ^ inverse);
}
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
add a comment |
up vote
4
down vote
up vote
4
down vote
You want to increase the tracker if one of src == left
or inverse
is true but not the other, and decrease it otherwise, which is what the "XOR" ^
operator does :
function handleDirection(src) {
if (src === 'left' ^ inverse) {
tracker++;
} else {
tracker--;
}
}
You can reduce that further by using a ternary expression :
function handleDirection(src) {
tracker += src === 'left' ^ inverse ? 1 : -1;
}
Or if you want to avoid any kind of conditionnal, with implicit casts and "clever" arithmetics :
function handleDirection(src) {
tracker += 1 - 2 * (src === 'right' ^ inverse);
}
You want to increase the tracker if one of src == left
or inverse
is true but not the other, and decrease it otherwise, which is what the "XOR" ^
operator does :
function handleDirection(src) {
if (src === 'left' ^ inverse) {
tracker++;
} else {
tracker--;
}
}
You can reduce that further by using a ternary expression :
function handleDirection(src) {
tracker += src === 'left' ^ inverse ? 1 : -1;
}
Or if you want to avoid any kind of conditionnal, with implicit casts and "clever" arithmetics :
function handleDirection(src) {
tracker += 1 - 2 * (src === 'right' ^ inverse);
}
edited 11 hours ago
answered 12 hours ago
Aaron
15.1k11636
15.1k11636
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
add a comment |
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
1
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
11 hours ago
1
1
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
@JacobRaihle thanks, I fixed the backward logic. The quotes around "clever" for the third example are sarcasm quotes, I wouldn't recommend using it unless the only point is to play the smartass.
– Aaron
11 hours ago
add a comment |
up vote
3
down vote
Assuming inverse
is a flag you'd set once, then you don't need to take it into account every time, you can calculate its impact once and just use it as it is, which will cut down your code branches and logic. If you want to change it as you go along, then you might need to separate the logic for the calculation, in order to re-use it.
You can also then extract the movement direction into a self-contained function and your handleDirection
becomes very simple - you calculate the direction you want to go based on src
and the invert
.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
With that said, all this suggests you shouldn't be using a function, or you should be using it differently. You can collect all that functionality in a class or instead have all the information passed around functions, so you don't have globals. Here is a sample object oriented implementation of the concept:
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
By the way, another alternative is to NOT compute the movement every time. You actually know what is happening every time - in effect, you have a truth table where your inputs are src === left
and inverse
and the outputs are how you modify your tracking.
+--------+------------+--------+
| isLeft | isInverted | Offset |
+--------+------------+--------+
| true | true | -1 |
| true | false | 1 |
| false | true | 1 |
| false | false | -1 |
+--------+------------+--------+
So, you can just put that table in.
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
In this case it might be an overkill but this approach might be useful if there are more flags (including more values for the flags) and/or end states. For example, maybe you want to introduce four directions, but you don't modify the tracker
value if it's up
or down
.
+-----------+------------+--------+
| direction | isInverted | Offset |
+-----------+------------+--------+
| left | true | -1 |
| left | false | 1 |
| right | true | 1 |
| right | false | -1 |
| up | false | 0 |
| up | true | 0 |
| down | false | 0 |
| down | true | 0 |
+-----------+------------+--------+
As you can see, now it's not just booleans, you can handle any value. Using a table, you also then change invert
to be something like windDirection
, so if the movement is left
and the windDirection
is right
, the result is like what it is now, but you could have direction of left
and wind going left
, so you move further. Or you can move up
and the wind direction is left
so tracker
(at this point the X coordinates) is going to actually be modified.
+-----------+---------------+---------+
| direction | windDirection | OffsetX |
+-----------+---------------+---------+
| left | right | -1 |
| left | up | 1 |
| left | down | 1 |
| left | left | 2 |
| right | up | -1 |
| right | down | -1 |
| right | right | -2 |
| right | left | 1 |
| up | up | 0 |
| up | down | 0 |
| up | left | 1 |
| up | right | -1 |
| down | up | 0 |
| down | down | 0 |
| down | left | 1 |
| down | right | -1 |
+-----------+---------------+---------+
With four directions and four wind directions to take into account the logic can be quite annoying to both read and maintain in the future, while if you only have a lookup table, it's easy and you can easily extend this to even handle diagonals (let's assume they change the value by 0.5
instead of 1
) and your algorithm would not really care as long as you just fetch the values from the table.
add a comment |
up vote
3
down vote
Assuming inverse
is a flag you'd set once, then you don't need to take it into account every time, you can calculate its impact once and just use it as it is, which will cut down your code branches and logic. If you want to change it as you go along, then you might need to separate the logic for the calculation, in order to re-use it.
You can also then extract the movement direction into a self-contained function and your handleDirection
becomes very simple - you calculate the direction you want to go based on src
and the invert
.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
With that said, all this suggests you shouldn't be using a function, or you should be using it differently. You can collect all that functionality in a class or instead have all the information passed around functions, so you don't have globals. Here is a sample object oriented implementation of the concept:
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
By the way, another alternative is to NOT compute the movement every time. You actually know what is happening every time - in effect, you have a truth table where your inputs are src === left
and inverse
and the outputs are how you modify your tracking.
+--------+------------+--------+
| isLeft | isInverted | Offset |
+--------+------------+--------+
| true | true | -1 |
| true | false | 1 |
| false | true | 1 |
| false | false | -1 |
+--------+------------+--------+
So, you can just put that table in.
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
In this case it might be an overkill but this approach might be useful if there are more flags (including more values for the flags) and/or end states. For example, maybe you want to introduce four directions, but you don't modify the tracker
value if it's up
or down
.
+-----------+------------+--------+
| direction | isInverted | Offset |
+-----------+------------+--------+
| left | true | -1 |
| left | false | 1 |
| right | true | 1 |
| right | false | -1 |
| up | false | 0 |
| up | true | 0 |
| down | false | 0 |
| down | true | 0 |
+-----------+------------+--------+
As you can see, now it's not just booleans, you can handle any value. Using a table, you also then change invert
to be something like windDirection
, so if the movement is left
and the windDirection
is right
, the result is like what it is now, but you could have direction of left
and wind going left
, so you move further. Or you can move up
and the wind direction is left
so tracker
(at this point the X coordinates) is going to actually be modified.
+-----------+---------------+---------+
| direction | windDirection | OffsetX |
+-----------+---------------+---------+
| left | right | -1 |
| left | up | 1 |
| left | down | 1 |
| left | left | 2 |
| right | up | -1 |
| right | down | -1 |
| right | right | -2 |
| right | left | 1 |
| up | up | 0 |
| up | down | 0 |
| up | left | 1 |
| up | right | -1 |
| down | up | 0 |
| down | down | 0 |
| down | left | 1 |
| down | right | -1 |
+-----------+---------------+---------+
With four directions and four wind directions to take into account the logic can be quite annoying to both read and maintain in the future, while if you only have a lookup table, it's easy and you can easily extend this to even handle diagonals (let's assume they change the value by 0.5
instead of 1
) and your algorithm would not really care as long as you just fetch the values from the table.
add a comment |
up vote
3
down vote
up vote
3
down vote
Assuming inverse
is a flag you'd set once, then you don't need to take it into account every time, you can calculate its impact once and just use it as it is, which will cut down your code branches and logic. If you want to change it as you go along, then you might need to separate the logic for the calculation, in order to re-use it.
You can also then extract the movement direction into a self-contained function and your handleDirection
becomes very simple - you calculate the direction you want to go based on src
and the invert
.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
With that said, all this suggests you shouldn't be using a function, or you should be using it differently. You can collect all that functionality in a class or instead have all the information passed around functions, so you don't have globals. Here is a sample object oriented implementation of the concept:
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
By the way, another alternative is to NOT compute the movement every time. You actually know what is happening every time - in effect, you have a truth table where your inputs are src === left
and inverse
and the outputs are how you modify your tracking.
+--------+------------+--------+
| isLeft | isInverted | Offset |
+--------+------------+--------+
| true | true | -1 |
| true | false | 1 |
| false | true | 1 |
| false | false | -1 |
+--------+------------+--------+
So, you can just put that table in.
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
In this case it might be an overkill but this approach might be useful if there are more flags (including more values for the flags) and/or end states. For example, maybe you want to introduce four directions, but you don't modify the tracker
value if it's up
or down
.
+-----------+------------+--------+
| direction | isInverted | Offset |
+-----------+------------+--------+
| left | true | -1 |
| left | false | 1 |
| right | true | 1 |
| right | false | -1 |
| up | false | 0 |
| up | true | 0 |
| down | false | 0 |
| down | true | 0 |
+-----------+------------+--------+
As you can see, now it's not just booleans, you can handle any value. Using a table, you also then change invert
to be something like windDirection
, so if the movement is left
and the windDirection
is right
, the result is like what it is now, but you could have direction of left
and wind going left
, so you move further. Or you can move up
and the wind direction is left
so tracker
(at this point the X coordinates) is going to actually be modified.
+-----------+---------------+---------+
| direction | windDirection | OffsetX |
+-----------+---------------+---------+
| left | right | -1 |
| left | up | 1 |
| left | down | 1 |
| left | left | 2 |
| right | up | -1 |
| right | down | -1 |
| right | right | -2 |
| right | left | 1 |
| up | up | 0 |
| up | down | 0 |
| up | left | 1 |
| up | right | -1 |
| down | up | 0 |
| down | down | 0 |
| down | left | 1 |
| down | right | -1 |
+-----------+---------------+---------+
With four directions and four wind directions to take into account the logic can be quite annoying to both read and maintain in the future, while if you only have a lookup table, it's easy and you can easily extend this to even handle diagonals (let's assume they change the value by 0.5
instead of 1
) and your algorithm would not really care as long as you just fetch the values from the table.
Assuming inverse
is a flag you'd set once, then you don't need to take it into account every time, you can calculate its impact once and just use it as it is, which will cut down your code branches and logic. If you want to change it as you go along, then you might need to separate the logic for the calculation, in order to re-use it.
You can also then extract the movement direction into a self-contained function and your handleDirection
becomes very simple - you calculate the direction you want to go based on src
and the invert
.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
With that said, all this suggests you shouldn't be using a function, or you should be using it differently. You can collect all that functionality in a class or instead have all the information passed around functions, so you don't have globals. Here is a sample object oriented implementation of the concept:
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
By the way, another alternative is to NOT compute the movement every time. You actually know what is happening every time - in effect, you have a truth table where your inputs are src === left
and inverse
and the outputs are how you modify your tracking.
+--------+------------+--------+
| isLeft | isInverted | Offset |
+--------+------------+--------+
| true | true | -1 |
| true | false | 1 |
| false | true | 1 |
| false | false | -1 |
+--------+------------+--------+
So, you can just put that table in.
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
In this case it might be an overkill but this approach might be useful if there are more flags (including more values for the flags) and/or end states. For example, maybe you want to introduce four directions, but you don't modify the tracker
value if it's up
or down
.
+-----------+------------+--------+
| direction | isInverted | Offset |
+-----------+------------+--------+
| left | true | -1 |
| left | false | 1 |
| right | true | 1 |
| right | false | -1 |
| up | false | 0 |
| up | true | 0 |
| down | false | 0 |
| down | true | 0 |
+-----------+------------+--------+
As you can see, now it's not just booleans, you can handle any value. Using a table, you also then change invert
to be something like windDirection
, so if the movement is left
and the windDirection
is right
, the result is like what it is now, but you could have direction of left
and wind going left
, so you move further. Or you can move up
and the wind direction is left
so tracker
(at this point the X coordinates) is going to actually be modified.
+-----------+---------------+---------+
| direction | windDirection | OffsetX |
+-----------+---------------+---------+
| left | right | -1 |
| left | up | 1 |
| left | down | 1 |
| left | left | 2 |
| right | up | -1 |
| right | down | -1 |
| right | right | -2 |
| right | left | 1 |
| up | up | 0 |
| up | down | 0 |
| up | left | 1 |
| up | right | -1 |
| down | up | 0 |
| down | down | 0 |
| down | left | 1 |
| down | right | -1 |
+-----------+---------------+---------+
With four directions and four wind directions to take into account the logic can be quite annoying to both read and maintain in the future, while if you only have a lookup table, it's easy and you can easily extend this to even handle diagonals (let's assume they change the value by 0.5
instead of 1
) and your algorithm would not really care as long as you just fetch the values from the table.
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
let tracker = 0;
//extract logic for the movement offset based on direction
function getDirectionOffset(src) {
return src === 'left' ? 1 : -1;
}
//have a setter for the invert property
function setInverse(isInverse) {
movementModifier = isInverse ? -1 : 1
}
//declare the variable dependent on the inverse property
let movementModifier;
//initialise movementModifier variable
setInverse(false);
function handleDirection(src) {
const offset = getDirectionOffset(src) * movementModifier;
tracker += offset;
}
// usage
setInverse(true);
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
class TrackerMover {
constructor(inverse) {
this.tracker = 0;
this.movementModifier = inverse ? 1 : -1
}
handleDirection(src) {
const offset = this.getDirectionOffset(src) * this.movementModifier;
this.tracker += offset;
}
getDirectionOffset(src) {
return src === 'left' ? -1 : 1;
}
getPosition() {
return this.tracker;
}
}
//usage
const mover = new TrackerMover(true);
mover.handleDirection("left");
mover.handleDirection("left");
mover.handleDirection("right");
console.log(mover.getPosition())
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
let tracker = 0;
let invert = false;
const movementLookupTable = {
"true": { },
"false": { },
}
//it can be initialised as part of the above expression but this is more readable
movementLookupTable[true ][true ] = -1;
movementLookupTable[true ][false] = 1;
movementLookupTable[false][true ] = 1;
movementLookupTable[false][false] = -1;
function handleDirection(src) {
const offset = movementLookupTable[src === "left"][invert];
tracker += offset;
}
// usage
invert = true;
handleDirection("left");
handleDirection("left");
handleDirection("right");
console.log(tracker);
edited 14 hours ago
answered 17 hours ago
vlaz
4,02921930
4,02921930
add a comment |
add a comment |
up vote
1
down vote
Right now you are comparing on strings, which I wouldn't advise. If for example you use 'Left' instead of 'left' it will fail the first if statement. Perhaps a boolean could be of use here, since you can guarantee it only has two states.
The if statements inside can be compressed via conditional operators.
Perhaps something like this is what you are looking for:
function handleDirection(src) {
if (src) {
inverse ? tracker-- : tracker++;
} else {
inverse ? tracker-- : tracker++;
}
}
See: https://jsfiddle.net/9zr4f3nv/
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
add a comment |
up vote
1
down vote
Right now you are comparing on strings, which I wouldn't advise. If for example you use 'Left' instead of 'left' it will fail the first if statement. Perhaps a boolean could be of use here, since you can guarantee it only has two states.
The if statements inside can be compressed via conditional operators.
Perhaps something like this is what you are looking for:
function handleDirection(src) {
if (src) {
inverse ? tracker-- : tracker++;
} else {
inverse ? tracker-- : tracker++;
}
}
See: https://jsfiddle.net/9zr4f3nv/
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
add a comment |
up vote
1
down vote
up vote
1
down vote
Right now you are comparing on strings, which I wouldn't advise. If for example you use 'Left' instead of 'left' it will fail the first if statement. Perhaps a boolean could be of use here, since you can guarantee it only has two states.
The if statements inside can be compressed via conditional operators.
Perhaps something like this is what you are looking for:
function handleDirection(src) {
if (src) {
inverse ? tracker-- : tracker++;
} else {
inverse ? tracker-- : tracker++;
}
}
See: https://jsfiddle.net/9zr4f3nv/
Right now you are comparing on strings, which I wouldn't advise. If for example you use 'Left' instead of 'left' it will fail the first if statement. Perhaps a boolean could be of use here, since you can guarantee it only has two states.
The if statements inside can be compressed via conditional operators.
Perhaps something like this is what you are looking for:
function handleDirection(src) {
if (src) {
inverse ? tracker-- : tracker++;
} else {
inverse ? tracker-- : tracker++;
}
}
See: https://jsfiddle.net/9zr4f3nv/
answered 17 hours ago
MagicLegend
106112
106112
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
add a comment |
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
3
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
11 hours ago
add a comment |
up vote
1
down vote
You can use short circuiting syntax or ternary operators
// by using short circuiting
function handleDirection(src) {
if (src == 'left') tracker = inverse && tracker-1 || tracker +1
else tracker = inverse && tracker+1 || tracker -1
}
// by using ternary operator
function handleDirection(src) {
if (src == 'left') tracker = inverse ? tracker-1 : tracker +1
else tracker = inverse ? tracker+1 : tracker -1
}
add a comment |
up vote
1
down vote
You can use short circuiting syntax or ternary operators
// by using short circuiting
function handleDirection(src) {
if (src == 'left') tracker = inverse && tracker-1 || tracker +1
else tracker = inverse && tracker+1 || tracker -1
}
// by using ternary operator
function handleDirection(src) {
if (src == 'left') tracker = inverse ? tracker-1 : tracker +1
else tracker = inverse ? tracker+1 : tracker -1
}
add a comment |
up vote
1
down vote
up vote
1
down vote
You can use short circuiting syntax or ternary operators
// by using short circuiting
function handleDirection(src) {
if (src == 'left') tracker = inverse && tracker-1 || tracker +1
else tracker = inverse && tracker+1 || tracker -1
}
// by using ternary operator
function handleDirection(src) {
if (src == 'left') tracker = inverse ? tracker-1 : tracker +1
else tracker = inverse ? tracker+1 : tracker -1
}
You can use short circuiting syntax or ternary operators
// by using short circuiting
function handleDirection(src) {
if (src == 'left') tracker = inverse && tracker-1 || tracker +1
else tracker = inverse && tracker+1 || tracker -1
}
// by using ternary operator
function handleDirection(src) {
if (src == 'left') tracker = inverse ? tracker-1 : tracker +1
else tracker = inverse ? tracker+1 : tracker -1
}
answered 17 hours ago
Komal Bansal
494
494
add a comment |
add a comment |
up vote
1
down vote
Not the shortest possible, but hopefully clear:
const LEFT = 'left'
const RIGHT = 'right'
function handleMovement(src) {
tracker += movementIncrement(src, inverse)
}
function movementIncrement(direction, inverse) {
if (inverse) {
direction = inverseDirection(direction)
}
return LEFT ? -1 : +1
}
function inverseDirection(direction) {
return direction === LEFT ? RIGHT : LEFT
}
add a comment |
up vote
1
down vote
Not the shortest possible, but hopefully clear:
const LEFT = 'left'
const RIGHT = 'right'
function handleMovement(src) {
tracker += movementIncrement(src, inverse)
}
function movementIncrement(direction, inverse) {
if (inverse) {
direction = inverseDirection(direction)
}
return LEFT ? -1 : +1
}
function inverseDirection(direction) {
return direction === LEFT ? RIGHT : LEFT
}
add a comment |
up vote
1
down vote
up vote
1
down vote
Not the shortest possible, but hopefully clear:
const LEFT = 'left'
const RIGHT = 'right'
function handleMovement(src) {
tracker += movementIncrement(src, inverse)
}
function movementIncrement(direction, inverse) {
if (inverse) {
direction = inverseDirection(direction)
}
return LEFT ? -1 : +1
}
function inverseDirection(direction) {
return direction === LEFT ? RIGHT : LEFT
}
Not the shortest possible, but hopefully clear:
const LEFT = 'left'
const RIGHT = 'right'
function handleMovement(src) {
tracker += movementIncrement(src, inverse)
}
function movementIncrement(direction, inverse) {
if (inverse) {
direction = inverseDirection(direction)
}
return LEFT ? -1 : +1
}
function inverseDirection(direction) {
return direction === LEFT ? RIGHT : LEFT
}
answered 12 hours ago
Anders
5,43463052
5,43463052
add a comment |
add a comment |
up vote
1
down vote
This has only one conditional, and I find it reads more intuitively than the other answers:
function handleDirection(src) {
if (
((src === 'left') && !inverse) ||
((src === 'right') && inverse)
) {
tracker++;
}
else {
tracker--;
}
}
add a comment |
up vote
1
down vote
This has only one conditional, and I find it reads more intuitively than the other answers:
function handleDirection(src) {
if (
((src === 'left') && !inverse) ||
((src === 'right') && inverse)
) {
tracker++;
}
else {
tracker--;
}
}
add a comment |
up vote
1
down vote
up vote
1
down vote
This has only one conditional, and I find it reads more intuitively than the other answers:
function handleDirection(src) {
if (
((src === 'left') && !inverse) ||
((src === 'right') && inverse)
) {
tracker++;
}
else {
tracker--;
}
}
This has only one conditional, and I find it reads more intuitively than the other answers:
function handleDirection(src) {
if (
((src === 'left') && !inverse) ||
((src === 'right') && inverse)
) {
tracker++;
}
else {
tracker--;
}
}
answered 12 hours ago
Paul S
1477
1477
add a comment |
add a comment |
up vote
1
down vote
I dislike elses and try to avoid nesting if possible. I think this conveys the idea of inverse
in a more natural way:
function handleDirection(src)
{
let change = 1;
if ('right' == src)
change = -1;
if (inverse)
change = -change;
tracker += change;
}
add a comment |
up vote
1
down vote
I dislike elses and try to avoid nesting if possible. I think this conveys the idea of inverse
in a more natural way:
function handleDirection(src)
{
let change = 1;
if ('right' == src)
change = -1;
if (inverse)
change = -change;
tracker += change;
}
add a comment |
up vote
1
down vote
up vote
1
down vote
I dislike elses and try to avoid nesting if possible. I think this conveys the idea of inverse
in a more natural way:
function handleDirection(src)
{
let change = 1;
if ('right' == src)
change = -1;
if (inverse)
change = -change;
tracker += change;
}
I dislike elses and try to avoid nesting if possible. I think this conveys the idea of inverse
in a more natural way:
function handleDirection(src)
{
let change = 1;
if ('right' == src)
change = -1;
if (inverse)
change = -change;
tracker += change;
}
answered 11 hours ago
Džuris
351726
351726
add a comment |
add a comment |
up vote
-1
down vote
You could use an 2 dimensional array type data structure from js and store the desired outcomes at index sec and inverse. Or JSON.
add a comment |
up vote
-1
down vote
You could use an 2 dimensional array type data structure from js and store the desired outcomes at index sec and inverse. Or JSON.
add a comment |
up vote
-1
down vote
up vote
-1
down vote
You could use an 2 dimensional array type data structure from js and store the desired outcomes at index sec and inverse. Or JSON.
You could use an 2 dimensional array type data structure from js and store the desired outcomes at index sec and inverse. Or JSON.
answered 11 hours ago
marshal craft
308314
308314
add a comment |
add a comment |
8
There's now a range of answers - one thing to bear in mind with this sort of thing is maintainability, that includes whether you yourself will understand what this code does next week. Make sure you pick a form of logic that is clear to you what it's doing at a glance - if that's the long form in your original question, stick with it.
– James Thorpe
17 hours ago
12
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
17 hours ago
8
Side note: your function uses 3 variables (
src
,inverse
andtracker
) but it has only 1 parameter (src
) and no return value. For that reason it would not pass my code review, regardless of how you structure theif
s....– Peter B
17 hours ago
1
@PeterB I'd generally agree, but it's worth noting that context is key. If this were a method in an object, then it might be fine. This could be manipulating some sort of cursor (
tracker
) via commands ("left"
/"right"
), the object itself has a flag that it would be moved in the opposite direction (invert
). However, as a free-floating function, that's indeed bad, as you're manipulating some not necessarily related global states.– vlaz
17 hours ago
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
17 hours ago