Setting flags to show three buttons

Question

I have this loop that iterates and assigns a variable to true depending on the different conditions

for (const element of actionsReferences) {
  if (element === 'accept') {
    this.showAcceptButton = true
  } else if (element === 'reject') {
    this.showRejectButton = true
  } else if (element === 'transfer') {
    this.showTransferButton = true
  }
}

How can i get the same result by avoiding if () ?

Several showXButton variables indicate a deeper code stink. You might want to show more of your code so that the core issue can be fixed. — Guy Incognito
– Guy Incognito, Commented Dec 8, 2017 at 8:46
An if/else like that should normally be a switch statement anyway — user98809
– user98809, Commented Dec 8, 2017 at 9:05
Your question has been migrated from Stack Overflow to Code Review. Here, we advise you to show some more code, so that we see exactly what this code is for and give you the best advice possible. Perhaps we could even eliminate these assignments altogether? See How to Ask. — 200_success
– 200_success, Commented Dec 8, 2017 at 12:47

Vogel612 · Accepted Answer · 2018-11-21 11:33:08Z

5

You could use a string to function "map", in JavaScript that can be implemented with a simple object:

var map = {
  'accept'   : function(o) {  o.showAcceptButton = true; },
  'reject'   : function(o) {  o.showRejectButton = true; },
  'transfer' : function(o) {  o.showTransferButton = true; }
};

let thisObject = {}; // fake this object

map['accept'](thisObject);
map[element](this); // use within your loop


// ES6 map
const map6 = {
    accept   : (o) => o.showAcceptButton = true,
    reject   : (o) => o.showRejectButton = true,
    transfer : (o) => o.showTransferButton = true
};

// alternative ES6 map
const map6a = {
    accept(o)   { o.showAcceptButton = true; },
    reject(o)   { o.showRejectButton = true; },
    transfer(o) { o.showTransferButton = true; }
};

map6['reject'](thisObject);
map6a['transfer'](thisObject);

// check if function exists and really is a function
if ('accept' in map6 && typeof map6['accept'] === 'function') map6['accept'](thisObject);

edited Nov 21, 2018 at 11:33

Vogel612

25.6k7 gold badges60 silver badges142 bronze badges

answered Dec 8, 2017 at 8:40

xander

2007 bronze badges

2

\$\begingroup\$ Improve is very subjective... I see this and think its just overcomplicating things. Still cool though ;) \$\endgroup\$

Ethan
– Ethan

2017-12-08 10:32:46 +00:00
Commented Dec 8, 2017 at 10:32
3

\$\begingroup\$ Downvoting this answer since it adds significant complexity and opacity for no measurable benefit. \$\endgroup\$

gntskn
– gntskn

2017-12-08 13:52:14 +00:00
Commented Dec 8, 2017 at 13:52
1

\$\begingroup\$ This is way too bloated. See Booligoosh's answer, this is the simplest way and is what I would personally have suggested. \$\endgroup\$

micheal65536
– micheal65536

2017-12-09 16:03:17 +00:00
Commented Dec 9, 2017 at 16:03
\$\begingroup\$ Couldn't you use setAttribute instead of defining 3 functions? \$\endgroup\$

Eric Duminil
– Eric Duminil

2017-12-09 16:28:08 +00:00
Commented Dec 9, 2017 at 16:28

Add a comment |

Salman Arshad · Accepted Answer · 2017-12-08 08:54:20Z

21

You could take an object and check if the action exists. If so, take the value as key for assignment.

const actions = {
        accept: 'showAcceptButton',
        reject: 'showRejectButton',
        transfer: 'showTransferButton'
    };

for (const element of actionsReferences) {
    if (element in actions) {
        this[actions[element]] = true;
    }
}

edited Dec 8, 2017 at 8:54

Salman Arshad

1033 bronze badges

answered Dec 8, 2017 at 8:39

Nina Scholz

7394 silver badges10 bronze badges

\$\begingroup\$ The best and most concise answer here :) \$\endgroup\$

Ethan
– Ethan

2017-12-08 10:33:02 +00:00
Commented Dec 8, 2017 at 10:33
1

\$\begingroup\$ -1 because it uses the this token which represents a security risk in open environments. \$\endgroup\$

Blindman67
– Blindman67

2017-12-08 11:16:44 +00:00
Commented Dec 8, 2017 at 11:16
4

\$\begingroup\$ @Blindman67, it's not my use of this. and this has nothing to do with a shorter proposal. \$\endgroup\$

Nina Scholz
– Nina Scholz

2017-12-08 11:17:59 +00:00
Commented Dec 8, 2017 at 11:17
\$\begingroup\$ I am well aware it is not your use this I think your exelent answer is not as exelent an answer as the accepted answer. Your answer is not as flexible as the functional approch and I want votes to go to the better answer. What if on of the buttons required an additional action. Code already in place is usually adapted in light of new requirements rather than re-written which would degrade the quality of you code. The functional approch can handle unique actions without addition logic. \$\endgroup\$

Blindman67
– Blindman67

2017-12-08 12:28:50 +00:00
Commented Dec 8, 2017 at 12:28

Add a comment |

Ethan · Accepted Answer · 2017-12-09 01:15:16Z

5

You can simply use this code:

for (const element of actionsReferences) {
  this.showAcceptButton = element === 'accept';
  this.showRejectButton = element === 'reject';
  this.showTransferButton = element === 'transfer';
}

Or, if you want the variables to stay as what they were set to if it returns false, use this:

for (const element of actionsReferences) {
  this.showAcceptButton = element === 'accept' || this.showAcceptButton;
  this.showRejectButton = element === 'reject' || this.showRejectButton;
  this.showTransferButton = element === 'transfer' || this.showTransferButton;
}

edited Dec 9, 2017 at 1:15

answered Dec 8, 2017 at 8:38

Ethan

1506 bronze badges

\$\begingroup\$ Was about to add this answer. The one thing I'd change is to add brackets to make it quicker for human to parse. this.showAcceptButton = (element === 'accept'); \$\endgroup\$

Peter
– Peter

2017-12-08 12:02:18 +00:00
Commented Dec 8, 2017 at 12:02
\$\begingroup\$ The first part of this answer is probably the best representation of what OP wanted in this entire thread; the second part could be improved by using || rather than a ternary: this.showAcceptButton = element === 'accept' || this.showAcceptButton \$\endgroup\$

gntskn
– gntskn

2017-12-08 13:55:44 +00:00
Commented Dec 8, 2017 at 13:55
\$\begingroup\$ The first snippet is wrong because only at most one button will be visible in the end. The second example contains an obvious code smell, as already pointed out in the comment above. \$\endgroup\$

xehpuk
– xehpuk

2017-12-08 15:51:22 +00:00
Commented Dec 8, 2017 at 15:51
1

\$\begingroup\$ @Carl these are not if else branches, they're === and || operations. The difference being that a) they do not accept any blocks, and are therefore a simpler and more primitive language structure and b) the compiler can (to some extent, since we're dynamic here) simply use CMP and OR instructions, which are simple math operations that do not require any branching at all. \$\endgroup\$

gntskn
– gntskn

2017-12-15 10:30:40 +00:00
Commented Dec 15, 2017 at 10:30
1

\$\begingroup\$ @Carl I now see that this comment was left before the answer was updated to use || over ternaries; I'll leave my comment in case anyone happens to have my imagined criticism, however :p \$\endgroup\$

gntskn
– gntskn

2017-12-15 10:32:29 +00:00
Commented Dec 15, 2017 at 10:32

| Show 1 more comment

Mihai Alexandru-Ionut · Accepted Answer · 2017-12-08 08:36:10Z

0

You can use bracket notation.

You have to capitalize first letter of element also.

element.charAt(0).toUpperCase() + element.slice(1)

Solution

for (const element of actionsReferences) {
   this['show' + element.charAt(0).toUpperCase() + element.slice(1) + 'Button'] = true
}

answered Dec 8, 2017 at 8:36

Mihai Alexandru-Ionut

1541 silver badge8 bronze badges

Add a comment |

gurvinder372 · Accepted Answer · 2017-12-08 08:37:39Z

0

Try bracket notation

var fnToFirstUpper = (str) => str.charAt(0).toUpperCase() + str.substring(1);

for (const element of actionsReferences) {
  this["show" + fnToFirstUpper( element ) + "Button" ] = true;
}

answered Dec 8, 2017 at 8:37

gurvinder372

1173 bronze badges

Add a comment |

Billal BEGUERADJ · Accepted Answer · 2017-12-08 10:05:23Z

0

You can use indexOf to achieve this:

var actionsReferences = ['accept', 'reject', 'transfer']
var showAcceptButton = actionsReferences.indexOf('accept') > -1;
var showRejectButton = actionsReferences.indexOf('reject')> -1;
var showTransferButton = actionsReferences.indexOf('transfer')> -1;

console.log(showAcceptButton);
console.log(showRejectButton);
console.log(showTransferButton);

edited Dec 8, 2017 at 10:05

Billal BEGUERADJ

3,56210 gold badges27 silver badges35 bronze badges

answered Dec 8, 2017 at 8:46

Vipin Kumar

1272 bronze badges

Add a comment |

Stack Exchange Network

Setting flags to show three buttons

6 Answers 6

You must log in to answer this question.

Hot Network Questions

Setting flags to show three buttons

6 Answers 6

You must log in to answer this question.

Related

Hot Network Questions