2
\$\begingroup\$

I am writing a program to combine the basic form of a word with suffix arrays following the choices from a form group (Angular 9) to receive the declination.

This is the flow:

  1. The user inputs a word.
  2. The user chooses the predefined grammar categories of the word (genus, class, animacy and others).
  3. According to the choices of the user the input is mapped with an array of the corresponding suffixes.

Now, my problem is that I can not wrap my head around the problem to find a more dynamic solution than writing an if-statement for every possible combination of choices and the corresponding array of suffixes.

Right now I compare the values array from the user choices with every predefined combination array. You will see in a second that this solution leads to a lot of repetitive code and I ask you to help me to find a better solution.

Two (of many possible) predefined conditions of choices plus the fitting suffix arrays

conditionOne = ['inanimate', 'm', '2', 'hard'];
conditionOneSuffixes = ['', 'а', 'у', 'а', 'ом', 'е', 'ы', 'ов', 'ам', 'ов', 'ами', 'ах'];
conditionTwo = ['animate', 'm', '2', 'hard'];
conditionTwoSuffixes = ['', 'а', 'у', 'а', 'ом', 'е', 'ы', 'ов', 'ам', 'ов', 'ами', 'ах'];

My first function to save the choices as values from angular reactive forms

setValues($event: string) {
    this.values = [];
    this.values = Object.values(this.substantiveFormGroup.value); // the user choices from the form group saved to an array 
    this.setDeclination(this.values);
  }

My second function to map the fitting suffix array on the input (only two possible if statements)

setDeclination(values) {
    if (
      this.conditionOne.every(i => values // the predefinded array of choices
        .includes(i))) {
      this.declination = this.conditionOneSuffixes.map( // the corresponding suffix array
        suffixes => this.substantiveFormGroup.get("input").value + suffixes
      );
    }
    else if (
      this.conditionTwo.every(i => values // another predefinded array of choices
        .includes(i))) {
      this.declination = this.conditionTwoSuffixes.map( // the corresponding suffix array
        suffixes => this.substantiveFormGroup.get("input").value + suffixes
      );
    }
// else if (another condition) { and so on...}
    else this.declination = [];
  }

What would be a better, less repetitive solution?

Thank you for your time.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

this.values = [] will never be used. It is unnecessary.

setValues($event: string) {
    //this.values = []; //unnecessary
    this.values = Object.values(this.substantiveFormGroup.value); // the user choices from the form group saved to an array 
    this.setDeclination(this.values);
  }

The code's logic will always set a value for this.declination, so it makes sense to just assign it at the top, so you know that this.declination will always be overwritten and by default should be initialized to an empty array. It also allows the code to be shorter.
If there will be more than two conditions, you should probably have them in a list anyways. This way of organizing still benefits, and the use of an array list and destructuring with for of loop expresses the code's use of conditions to check for and suffixes to use in an organized list in one place, as well as shortening the code significantly and making it more concise.
The result is that it can be more simply read as an iteration on a list of conditions to process.
Brevity and DRY naturally occurs when it is distilled this way.

setDeclination(values) {
    this.declination = [];
    for (const [condition, suffixes] of [
     [this.conditionOne, this.conditionOneSuffixes],
     [this.conditionTwo, this.conditionTwoSuffixes]
    ])
       if(condition.every(i => values.includes(i))) {
         const inputValue = this.substantiveFormGroup.get("input").value;
         this.declination = suffixes.map(suffix => inputValue + suffix));
         break;
       }
 }

I have made some modifications to the formatting and the declaration of variables to control line length and make the code more concise, and it should be mostly self-explanatory.

\$\endgroup\$
2
  • \$\begingroup\$ That looks fine. Thank you! \$\endgroup\$
    – Lachman
    May 26, 2020 at 12:00
  • \$\begingroup\$ Welcome to Code Review. At the moment you provided an alternate solution with no explanation or justification that may be deleted, to avoid this situation check for further details how-to -answer. \$\endgroup\$ May 26, 2020 at 12:21

Your Answer

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

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