1
\$\begingroup\$

I have an Angular 5 project with a method named createForm. It will basically create just a form. However, the form created changes depends on the flight inventory code. If flight inventory code id TAG_ON I will create a form omitting messagePrefixSMSValidator and also form control named message to empty.

createForm() {

    this.formGroup = this.fb.group(
      {
        defaultTemplate: [this.defaultInitialValue],
        language: [null, Validators.required],
        message: [ this.messagePrefix ? this.messagePrefix:'', [Validators.required]],
        longUrl: ['']
      },
      {
        validator: [
                    hasUrlTagValidator(TemplatesService.urlTag), 
                    messagePrefixSMSValidator(this.messagePrefix? this.messagePrefix: null, 'message')
                  ]
      }
    );

   if(this.flight.inventory.code === FlightInventoryCode.TAG_ON) {

    this.formGroup =  this.fb.group(
      {
        defaultTemplate: [this.defaultInitialValue],
        language: [null, Validators.required],
        message: [ '', [Validators.required]],
        longUrl: ['']
      },
      {
        validator: [
                    hasUrlTagValidator(TemplatesService.urlTag), 
                  ]
      }
    );
   }

  }

Is there a way to rewrite my code so that it looks simple?

\$\endgroup\$
0
\$\begingroup\$

Suggested changes

  • Formatting. Typically you want to use TSLint, and possible also Prettier. I've ran your code through Prettier with the standard configuration. (Note that the point is consistency and readability. It doesn't matter if you want to use ' or ", tabs or spaces, etc)
  • this.messagePrefix ? this.messagePrefix : "" can be simplified as this.messagePrefix || ""
  • Looking at your code, it looks like we want to assign different values to message and validator depending on the values of this.flight.inventory.code. So lets keep everything else the same rather than having separate code paths. If we were to keep the current structure, then we should at least put the first assignment to this.formGroup in an else block, so we don't assign it twice.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ i didnt understand the last point . could you clarify more , \$\endgroup\$ – knowledgeseeker Aug 1 '19 at 15:01
  • \$\begingroup\$ I think the best way to understand it is to compare the codepen I linked to your original code. Hopefully you can see the gain in readability and terseness. It's much more clear that message and validator (but nothing else) depends on this.flight.inventory.code in the codepen version. \$\endgroup\$ – Mohrn Aug 1 '19 at 15:08
  • 1
    \$\begingroup\$ thank you so much .. i saw it in codepen ...brilliant \$\endgroup\$ – knowledgeseeker Aug 1 '19 at 15:13
  • \$\begingroup\$ Great! Feel free to up vote or accept my answer if it helped you. \$\endgroup\$ – Mohrn Aug 1 '19 at 15:17
  • \$\begingroup\$ done . iforgot to voteup .thank u \$\endgroup\$ – knowledgeseeker Aug 1 '19 at 15:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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