3
\$\begingroup\$

Is there any approaches to reduce the below coding snippet in Angular 2+ with few lines of code by making use of ECMA Script or by some other ways

this.testCenterAccomodations.forEach((x: { utilityCode: string; isChecked: boolean; isSelected: boolean; }) => {
    if (x.utilityCode === 'PSDQRA') {
        if (this.voiceOverLanguageChecked) {
            x.isChecked = true;
            x.isSelected = true;
        } else {
            x.isChecked = false;
            x.isSelected = false;
        }
    }
});

Looping an Array of objects, we are trying to find the object with the column named utilityCode having PSDQRA value and setting those particular object's properties of isChecked & isSelected to be true iff, this.voiceOverLanguageChecked boolean value is true, otherwise set those properties to false.

The above snippets is simpler enough to read & understand. But is it possible for someone to achieve the same piece of logic with few lines of code by making use of map, filter, reduce, lambda and rest..

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Hmm i would in a first refactoring change it to this:

this.testCenterAccomodations.forEach((x: { utilityCode: string; isChecked: boolean; isSelected: boolean; }) => {
  if (x.utilityCode != 'PSDQRA') {
    return;
  };
  x.isChecked = this.voiceOverLanguageChecked;
  x.isSelected = this.voiceOverLanguageChecked;
});

The next changes do not have the goal of "shortening" the code. But to make it more readable and easier to change. I would love to hear your feedback about my thoughts.

I would create a small (perhaps local) interface, because { utilityCode: string; isChecked: boolean; isSelected: boolean; } seems to be a recurring thing.

interface UtilityCodeSelection { 
  utilityCode: string;
  isChecked: boolean; 
  isSelected: boolean; 
}

Which will result in

this.testCenterAccomodations.forEach((x: UtilityCodeSelection ) => {
  if (x.utilityCode != 'PSDQRA') {
    return;
  };
  x.isChecked = this.voiceOverLanguageChecked;
  x.isSelected = this.voiceOverLanguageChecked;
});

Then i would replace the magic variable "PSDQRA" by a constant (or by an enum if you have multiple utility codes we are coding against). Then we could use that constant at all necessary places and would be able to change it centraly. ALSO we can easily find out where our code is refering to this specific value.

interface UtilityCodeSelection { 
  utilityCode: string;
  isChecked: boolean; 
  isSelected: boolean; 
}

export enum UtilityCodes {
    PSDQRA = 'PSDQRA', // Or if PSDQRA has in reality a better name, use that for the enum
    ...
}

this.testCenterAccomodations.forEach((x: UtilityCodeSelection ) => {
  if (x.utilityCode != UtilityCodes.PSDQRA) {
    return;
  };
  x.isChecked = this.voiceOverLanguageChecked;
  x.isSelected = this.voiceOverLanguageChecked;
});

And in some cases i would even extract some parts into its own method

this.testCenterAccomodations.forEach((x: UtilityCodeSelection ) => this.setUtilityCodeSelection(x, UtilityCode.PSDQRA, this.voiceOverLanguageChecked));

 private setUtilityCodeSelection(code: UtilityCodeSelection, forCode: UtilityCode, setTo: boolean):void{
    if (code.utilityCode != forCode) {
      return;
    };
    code.isChecked = setTo;
    code.isSelected = setTo;
}

This has some drawbacks. I personaly do not like implict changes (here we change the given code) and prefere to change explicitly (make a copy, change the copy and return the copy). Also the extra method is only useful if there are more similar checks. In general its a bad idea to try to generalize things, if there is (at least now) no need for it.

Therefor the chance is high that in your example, i would not move the code in its own method..

As always... Ask 3 developers about coding style and you will get 4 answers. :-) And they all will be right in some way.

warm regards

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.