2
\$\begingroup\$

So I have following piece of code which basically validates five date fields in angular service.

But the catch is that there may be one to five field(s) visible on the screen based on some other criteria.So the code checks if one datepicker is shown it will only validate one input.If three datepickers are shown it would validate three inputs.

this.isDatesValid = function (tarifForm) {
     var valid = true;

            //check "Sinistres en 5 ans" is 1/2/3/4/5 then based on that check the datepickers should not be empty
     switch (Number(tarifForm.Attestation.sinistres)) {
            case 1:
                    if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
                        valid = false;
                        tarifForm.Attestation.firstSinisterDateinValid = true;
                    }
                    break;
            case 2:
                    if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
                        valid = false;
                        tarifForm.Attestation.firstSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr2nd.output == null) {
                        valid = false;
                        tarifForm.Attestation.secondSinisterDateinValid = true;
                    }
                    break;
            case 3:
                    if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
                        valid = false;
                        tarifForm.Attestation.firstSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr2nd.output == null) {
                        valid = false;
                        tarifForm.Attestation.secondSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr3rd.output == null) {
                        valid = false;
                        tarifForm.Attestation.thirdSinisterDateinValid = true;
                    }
                    break;
            case 4:
                    if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
                        valid = false;
                        tarifForm.Attestation.firstSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr2nd.output == null) {
                        valid = false;
                        tarifForm.Attestation.secondSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr3rd.output == null) {
                        valid = false;
                        tarifForm.Attestation.thirdSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr4th.output == null) {
                        valid = false;
                        tarifForm.Attestation.fourthSinisterDateinValid = true;
                    }
                    break;
             case 5:
                    if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
                        valid = false;
                        tarifForm.Attestation.firstSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr2nd.output == null) {
                        valid = false;
                        tarifForm.Attestation.secondSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr3rd.output == null) {
                        valid = false;
                        tarifForm.Attestation.thirdSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr4th.output == null) {
                        valid = false;
                        tarifForm.Attestation.fourthSinisterDateinValid = true;
                    }
                    if (tarifForm.Attestation.premiersinsiterdatepr5th.output == null) {
                        valid = false;
                        tarifForm.Attestation.fifthSinisterDateinValid = true;
                    }
                    break;
            }
            var inputdata = { sinistreEn5AnsAT: tarifForm.Attestation.sinistres, DateFifthSinister: commonUtilities.currentDateinDDMMYYYY(tarifForm.Attestation.premiersinsiterdatepr5th.output), DateFourthSinister: commonUtilities.currentDateinDDMMYYYY(tarifForm.Attestation.premiersinsiterdatepr4th.output), DateThirdSinister: commonUtilities.currentDateinDDMMYYYY(tarifForm.Attestation.premiersinsiterdatepr3rd.output), DateSecondSinister: commonUtilities.currentDateinDDMMYYYY(tarifForm.Attestation.premiersinsiterdatepr2nd.output), DateFirstSinister: commonUtilities.currentDateinDDMMYYYY(tarifForm.Attestation.premiersinsiterdatepr1st.output) };
            this.isValidDate(inputdata, function (response) {
                valid = Boolean(response.data);
            });
            return valid;
        }

In view validations are applied like this.I have only shown one case here.

 <div class="row">
      <div class="push-four field eight columns">
      <span class="msg-err" ng-show="tarifForm.Attestation.secondSinisterDateinValid" translate code="TarifficationErrorMsg$ErrorMsgRequiredCatalougeandInsured"></span>
      </div>
 </div>

My question is can I simplify the above if conditions written.My objective is to reduce the number of redundant code in different if blocks.

\$\endgroup\$

3 Answers 3

2
\$\begingroup\$
case 3:
        if (tarifForm.Attestation.premiersinsiterdatepr1st.output == null) {
            valid = false;
            tarifForm.Attestation.firstSinisterDateinValid = true;
        }
        if (tarifForm.Attestation.premiersinsiterdatepr2nd.output == null) {
            valid = false;
            tarifForm.Attestation.secondSinisterDateinValid = true;
        }
        if (tarifForm.Attestation.premiersinsiterdatepr3rd.output == null) {
            valid = false;
            tarifForm.Attestation.thirdSinisterDateinValid = true;
        }

These variable names are some of the most unreadable ones I've seen. My thought process…

  1. premiersinsiterdatepr1st: "Premiers insiter (insister?) date pr1st (priest?)"? Makes no sense. "Premier sin siter…?" Makes no sense. "Premier sin sit er date…?" Makes no sense. Grumble.
  2. Moving on to firstSinisterDateinValid… "first sinister Datein (Datei?) Valid"? Does this code have something to do with files?
  3. Second attempt at premiersinsiterdatepr1st: "Premier sinister (sinistre?) date priest?"
  4. After seeing …2nd and …3rd, reparse premiersinsiterdatepr1st as "Premier sinister (sinistre?) date pr (??) 1st"
  5. What does "pr" stand for? "premier"? "primaire"? "programme"? "procuration"? "prostitution"? "public relations"? "picker"? I still have absolutely no idea.

Please,

  • Long words work for languages like German and Finnish with simple spelling, but çanemarchepasenfrançais. In JavaScript, you need to useInterCaps.
  • Break words correctly. "Date Invalid" makes sense. "Datein Valid" does not.
  • Pick a language and stick with it. Why did you use "premier" in conjunction with "second" and "third"? Given a choice between French and English, you would be much better off with English only.
  • Spell correctly. "sinsiter" was misspelled every time!
\$\endgroup\$
1
\$\begingroup\$

Of course you can.

  1. You should create array as

    var dateValidationSettings = [{numberValue: 1, textValue: "first"}, ...{numberValue: 5, textValue: "fifth"}]
    
  2. Remove switch

  3. Use loop with array reflection (you can set some data to some property by name)

\$\endgroup\$
2
  • 1
    \$\begingroup\$ :thnx for response,can you pls elaborate little bit.My only concern is to reduce and generalise the if blocks . \$\endgroup\$
    – user59802
    Commented May 16, 2016 at 13:30
  • \$\begingroup\$ So, you create only one IF and one loop and one array. I sure this issue has a better solution, but my solution can be a life \$\endgroup\$
    – Mediator
    Commented May 17, 2016 at 5:55
0
\$\begingroup\$

If you are checking that these date fields are in the correct date format,

  1. Use some Jquery-UI and add a date mask, and then probably a DatePicker depending on what you are doing with these dates.
  2. Create a single method in your Angular controller to check for a valid date (null or other invalid format), let it return a boolean value for valid(true) or invalid(false)
  3. Create a variable in your controller to hold the forms Valid state
  4. Set the ng-blur attribute (of the date input) to show the error message element if the date input isn't valid upon focus being lost.

Then you can create a loop for the HTML side with Angular and the code will be much simpler.

\$\endgroup\$