Simplify an if/else statement? [closed]
up vote
21
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
closed as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed Dec 13 at 18:28
- 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
21
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
closed as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed Dec 13 at 18:28
- 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
Dec 13 at 10:31
13
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
9
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
Dec 13 at 10:34
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
Dec 13 at 10:44
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57
|
show 6 more comments
up vote
21
down vote
favorite
up vote
21
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 Dec 13 at 17:34
Charlie Harding
261414
261414
asked Dec 13 at 10:24
Rebecca O'Sullivan
411214
411214
closed as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed Dec 13 at 18:28
- 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.
closed as off-topic by Gabriele Petrioli, vlaz, BlueRaja - Danny Pflughoeft, Daniel, coldspeed Dec 13 at 18:28
- 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
Dec 13 at 10:31
13
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
9
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
Dec 13 at 10:34
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
Dec 13 at 10:44
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57
|
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
Dec 13 at 10:31
13
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
9
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
Dec 13 at 10:34
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
Dec 13 at 10:44
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57
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
Dec 13 at 10:31
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
Dec 13 at 10:31
13
13
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
9
9
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
Dec 13 at 10:34
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
Dec 13 at 10:34
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
Dec 13 at 10:44
@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
Dec 13 at 10:44
3
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57
|
show 6 more comments
12 Answers
12
active
oldest
votes
up vote
35
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++;
}
}
The check evaluates the expression in this order (src === 'left') === inverse
:
src === 'left' === inverse
---- first --- returns a boolean value
--------- second --------- take result of former check & compairs it with another boolean
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
1
@afe If you're doing it with the ternary operator, I think an increment (tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer
– Charlie Harding
Dec 13 at 17:05
|
show 8 more comments
up vote
12
down vote
function handleDirection(src) {
var movement = 1;
if(src === 'left')
movement = -1;
if(inverse)
tracker += movement;
else
tracker -= movement;
}
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
Dec 13 at 11:01
3
@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
Dec 13 at 16:41
add a comment |
up vote
9
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
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
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); // either 1-0=1 or 1-2=-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
Dec 13 at 17:08
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
Dec 13 at 17:18
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
2
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
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
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
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
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
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
35
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++;
}
}
The check evaluates the expression in this order (src === 'left') === inverse
:
src === 'left' === inverse
---- first --- returns a boolean value
--------- second --------- take result of former check & compairs it with another boolean
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
1
@afe If you're doing it with the ternary operator, I think an increment (tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer
– Charlie Harding
Dec 13 at 17:05
|
show 8 more comments
up vote
35
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++;
}
}
The check evaluates the expression in this order (src === 'left') === inverse
:
src === 'left' === inverse
---- first --- returns a boolean value
--------- second --------- take result of former check & compairs it with another boolean
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
1
@afe If you're doing it with the ternary operator, I think an increment (tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer
– Charlie Harding
Dec 13 at 17:05
|
show 8 more comments
up vote
35
down vote
accepted
up vote
35
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++;
}
}
The check evaluates the expression in this order (src === 'left') === inverse
:
src === 'left' === inverse
---- first --- returns a boolean value
--------- second --------- take result of former check & compairs it with another boolean
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++;
}
}
The check evaluates the expression in this order (src === 'left') === inverse
:
src === 'left' === inverse
---- first --- returns a boolean value
--------- second --------- take result of former check & compairs it with another boolean
edited 2 days ago
answered Dec 13 at 10:29
Nina Scholz
174k1387151
174k1387151
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
1
@afe If you're doing it with the ternary operator, I think an increment (tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer
– Charlie Harding
Dec 13 at 17:05
|
show 8 more comments
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
1
@afe If you're doing it with the ternary operator, I think an increment (tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer
– Charlie Harding
Dec 13 at 17:05
2
2
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
^^ only in this special case
– Nina Scholz
Dec 13 at 10:33
1
1
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
Ah, makes sense I suppose (bool) == (condition_met) .. +2 skill points to efficiency! thank you :)
– treyBake
Dec 13 at 10:40
6
6
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
I definitely agree with @marcelm. As wonderful as this answer looks, it is not immediately obvious what is happening.
– Marie
Dec 13 at 16:18
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
Dec 13 at 16:21
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
Dec 13 at 16:21
1
1
@afe If you're doing it with the ternary operator, I think an increment (
tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer– Charlie Harding
Dec 13 at 17:05
@afe If you're doing it with the ternary operator, I think an increment (
tracker += (src == 'left') == inverse ? -1 : +1;
) is a little clearer– Charlie Harding
Dec 13 at 17:05
|
show 8 more comments
up vote
12
down vote
function handleDirection(src) {
var movement = 1;
if(src === 'left')
movement = -1;
if(inverse)
tracker += movement;
else
tracker -= movement;
}
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
Dec 13 at 11:01
3
@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
Dec 13 at 16:41
add a comment |
up vote
12
down vote
function handleDirection(src) {
var movement = 1;
if(src === 'left')
movement = -1;
if(inverse)
tracker += movement;
else
tracker -= movement;
}
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
Dec 13 at 11:01
3
@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
Dec 13 at 16:41
add a comment |
up vote
12
down vote
up vote
12
down vote
function handleDirection(src) {
var movement = 1;
if(src === 'left')
movement = -1;
if(inverse)
tracker += movement;
else
tracker -= movement;
}
function handleDirection(src) {
var movement = 1;
if(src === 'left')
movement = -1;
if(inverse)
tracker += movement;
else
tracker -= movement;
}
edited Dec 14 at 15:26
answered Dec 13 at 10:30
Alays
3069
3069
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
Dec 13 at 11:01
3
@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
Dec 13 at 16:41
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
Dec 13 at 11:01
3
@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
Dec 13 at 16:41
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
Dec 13 at 11:01
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
Dec 13 at 11:01
3
3
@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
Dec 13 at 16:41
@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
Dec 13 at 16:41
add a comment |
up vote
9
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
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
add a comment |
up vote
9
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
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
add a comment |
up vote
9
down vote
up vote
9
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 Dec 13 at 12:20
Leuronics
912
912
New contributor
New contributor
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
add a comment |
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
1
1
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
Very nice solution!
– Julius Naeumann
Dec 14 at 12:52
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 Dec 13 at 10:34
answered Dec 13 at 10:29
Rory McCrossan
240k29205244
240k29205244
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); // either 1-0=1 or 1-2=-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
Dec 13 at 17:08
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
Dec 13 at 17:18
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); // either 1-0=1 or 1-2=-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
Dec 13 at 17:08
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
Dec 13 at 17:18
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); // either 1-0=1 or 1-2=-1
}
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); // either 1-0=1 or 1-2=-1
}
edited yesterday
answered Dec 13 at 16:17
Aaron
15.2k11636
15.2k11636
1
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
Dec 13 at 17:08
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
Dec 13 at 17:18
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
Dec 13 at 17:08
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
Dec 13 at 17:18
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
Dec 13 at 17:08
You got the logic backwards, and I hate the third example, but +1 for actually using the operator built for this.
– Jacob Raihle
Dec 13 at 17:08
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
Dec 13 at 17:18
@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
Dec 13 at 17:18
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 Dec 13 at 13:20
answered Dec 13 at 11:16
vlaz
4,03821930
4,03821930
add a comment |
add a comment |
up vote
2
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
2
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
2
down vote
up vote
2
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 Dec 13 at 16:06
Paul S
1577
1577
add a comment |
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 Dec 13 at 10:34
Komal Bansal
665
665
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 Dec 13 at 15:47
Anders
5,48463052
5,48463052
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 Dec 13 at 16:29
Džuris
351726
351726
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
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
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
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
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/
edited Dec 14 at 17:06
answered Dec 13 at 10:32
MagicLegend
106112
106112
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
add a comment |
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
3
3
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
Dec 13 at 17:03
Your two branches of the if statement are identical: one of them needs to be reversed
– Charlie Harding
Dec 13 at 17:03
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
Oopsie! You're right hehe. Corrected!
– MagicLegend
Dec 14 at 17:06
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 Dec 13 at 16:58
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
Dec 13 at 10:31
13
I'm voting to close this question as off-topic because it belongs to codereview.stackexchange.com
– Gabriele Petrioli
Dec 13 at 10:33
9
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
Dec 13 at 10:34
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
Dec 13 at 10:44
3
I think the main benefit of this question is just to let others earn more reputation more easily!
– lucumt
Dec 13 at 10:57