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\$

1 Answer 1

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.
\$\endgroup\$
5
  • \$\begingroup\$ i didnt understand the last point . could you clarify more , \$\endgroup\$ Aug 1, 2019 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, 2019 at 15:08
  • 1
    \$\begingroup\$ thank you so much .. i saw it in codepen ...brilliant \$\endgroup\$ Aug 1, 2019 at 15:13
  • \$\begingroup\$ Great! Feel free to up vote or accept my answer if it helped you. \$\endgroup\$
    – Mohrn
    Aug 1, 2019 at 15:17
  • \$\begingroup\$ done . iforgot to voteup .thank u \$\endgroup\$ Aug 1, 2019 at 15:27

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.