1
\$\begingroup\$

I have this working code which shows the progress bar based on different states ("not-started", "progressing" and "done"). Each state will have its own class.

I notice it is not Don't Repeat Yourself (DRY) enough but not sure how I can refactor it. Appreciate your code review.

new Vue({
  el: "#page",
  data: {
    currentStepNumber:10,
    totalEformStepNumber: 9,
    totalVerificationStepNumber: 2,
    totalReviewStepNumber: 1,
    eFormProgress: "not-started",
    verificationProgress: "not-started",
    reviewProgress: "not-started",
    verificationDone: false,
    encourageText: "Let's Start"
  },
  computed: {
    checkEFormProgress() {
      if (this.eFormProgress === "not-started") {
        return "step-indicator-not-started";
      } else if (this.eFormProgress === "progressing") {
        return "step-indicator-progressing";
      } else if (this.eFormProgress === "done") {
        return "step-indicator-done";
      }
    },
    checkVerificationProgress() {
      if (this.verificationProgress === "not-started") {
        return "step-indicator-not-started";
      } else if (this.verificationProgress === "progressing") {
        return "step-indicator-progressing";
      } else if (this.verificationProgress === "done") {
        return "step-indicator-done";
      }
    },
    checkReviewProgress() {
      if (this.reviewProgress === "not-started") {
        return "step-indicator-not-started";
      } else if (this.reviewProgress === "progressing") {
        return "step-indicator-progressing";
      } else if (this.reviewProgress === "done") {
        return "step-indicator-done";
      }
    },
    totalStepNumber() {
      return (
        this.totalEformStepNumber +
        this.totalVerificationStepNumber +
        this.totalReviewStepNumber
      );
    },
    eformCurrentStepNumber() {
      if (this.currentStepNumber === 0) {
        this.eFormProgress = "not-started";
      } else if (this.currentStepNumber < this.totalEformStepNumber) {
        this.eFormProgress = "progressing";
      } else if (this.currentStepNumber >= this.totalEformStepNumber) {
        this.eFormProgress = "done";
      }
      return (this.currentStepNumber / this.totalEformStepNumber) * 100;
    },
    verifcationCurrentStepNumber() {
      debugger;
      if (this.currentStepNumber >= this.totalEformStepNumber) {
        if (this.currentStepNumber < this.totalEformStepNumber) {
          this.verificationProgress = "not-started";
        } else if (this.currentStepNumber > this.totalEformStepNumber && this.currentStepNumber < this.totalStepNumber) {
          this.verificationProgress = "progressing";
        } else if (this.currentStepNumber === this.totalStepNumber) {
          this.verificationProgress = "done";          
          this.reviewProgress = "done";
        }
        return (
          ((this.currentStepNumber - this.totalEformStepNumber) /
            this.totalVerificationStepNumber) *
          100
        );
      } else {
        return 0;
      }
    },
    eformProgressWidth() {
      return this.eformCurrentStepNumber > 100
        ? { width: "100%" }
        : { width: this.eformCurrentStepNumber + "%" };
    },
    verificationProgressWidth() {
      return this.verifcationCurrentStepNumber >= 100
        ? { width: "100%" }
        : { width: this.verifcationCurrentStepNumber + "%" };
    }
  },
  methods: {},
  created() {}
});
@import url("https://fonts.googleapis.com/css2?family=Roboto&display=swap");
 p {
     margin: 0;
}
 #page {
     font-family: Roboto;
     width: 100%;
     background: #f8f9fa;
     display: flex;
     flex-direction: column;
     align-items: center;
}
 .container {
     width: 70%;
     text-align: center;
}
 .progress-bar {
     display: flex;
     flex-direction: row;
     align-items: flex-start;
     width: 100%;
}
 .point {
     width: 38px;
     display: flex;
     flex-direction: column;
     align-items: center;
     gap: 8px;
     color: #28a745;
}
 .step-indicator-not-started {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: none;
}
 .step-indicator-progressing {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: 4px solid #28a745;
}
 .step-indicator-done {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #28a745;
     box-sizing: border-box;
     border: 4px solid #28a745;
}
 .bar {
     width: 100%;
     height: 16px;
     display: grid;
     align-items: center;
}
 .bar-eform {
     width: 80%;
}
 .bar-verification {
     width: 20%;
}
 .bar-bg {
     height: 4px;
     background-color: #ecedee;
     position: relative;
}
 .bar-arrow {
     position: absolute;
     top: 0;
     left: 0;
     height: 4px;
     background-color: #28a745;
}
 .page-indicator {
     color: #28a745;
     font-size: 14px;
}
 
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/js/bootstrap.min.js"></script>
<div id="page">
  <div class="container">
    <h3>{{encourageText}}</h3>
    <div class="progress-bar">
      <div class="point">
        <div :class="checkEFormProgress">
        </div>
        <p><strong>EForm</strong></p>
      </div>
      <div class="bar bar-eform">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="eformProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="checkVerificationProgress">
        </div>
        <p><strong>Verification</strong></p>
      </div>
      <div class="bar bar-verification">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="verificationProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="checkReviewProgress">
        </div>
        <p><strong>Review</strong></p>
      </div>
    </div>
  </div>
  <div class="page-indicator">
    <strong>{{currentStepNumber}}/{{totalStepNumber}}</strong>
  </div>
</div>

Version 2, I have done some refactoring as below.

const stateProgress = {
  NOTSTARTED: "not-started",
  PROGRESSING: "progressing",
  DONE: "done"
};

const stateProgressClassName = {
  NOTSTARTED: "step-indicator-not-started",
  PROGRESSING: "step-indicator-progressing",
  DONE: "step-indicator-done"
};

new Vue({
  el: "#page",
  data: {
    currentStepNumber: 2,
    totalEformStepNumber: 9,
    totalVerificationStepNumber: 2,
    totalReviewStepNumber: 1,
    eFormProgress: stateProgress.NOTSTARTED,
    verificationProgress: stateProgress.NOTSTARTED,
    reviewProgress: stateProgress.NOTSTARTED,
    encourageText: "Let's Start"
  },
  computed: {
    checkEFormProgress() {
      return this.getProgressClassName(this.eFormProgress);      
    },
    checkVerificationProgress() {
      return this.getProgressClassName(this.verificationProgress);
    },
    checkReviewProgress() {
      return this.getProgressClassName(this.reviewProgress);
    },
    totalStepNumber() {
      return (
        this.totalEformStepNumber +
        this.totalVerificationStepNumber +
        this.totalReviewStepNumber
      );
    },
    eformCurrentStepNumber() {
      if (this.currentStepNumber <= 0) {
        this.eFormProgress = stateProgress.NOTSTARTED;
      } else if (this.currentStepNumber < this.totalEformStepNumber) {
        this.eFormProgress = stateProgress.PROGRESSING;
      } else if (this.currentStepNumber >= this.totalEformStepNumber) {
        this.eFormProgress = stateProgress.DONE;
      }
      return (this.currentStepNumber / this.totalEformStepNumber) * 100;
    },
    verifcationCurrentStepNumber() {
      debugger;
      if (this.currentStepNumber >= this.totalEformStepNumber) {
        if (this.currentStepNumber < this.totalEformStepNumber) {
          this.verificationProgress = stateProgress.NOTSTARTED;
        } else if (
          this.currentStepNumber > this.totalEformStepNumber &&
          this.currentStepNumber < this.totalStepNumber
        ) {
          this.verificationProgress = stateProgress.PROGRESSING;
        } else if (this.currentStepNumber === this.totalStepNumber) {
          this.verificationProgress = stateProgress.DONE;
          this.reviewProgress = stateProgress.PROGRESSING;
        }
        return (
          ((this.currentStepNumber - this.totalEformStepNumber) /
            this.totalVerificationStepNumber) *
          100
        );
      } else {
        return 0;
      }
    },
    eformProgressWidth() {
      return this.eformCurrentStepNumber > 100
        ? { width: "100%" }
        : { width: this.eformCurrentStepNumber + "%" };
    },
    verificationProgressWidth() {
      return this.verifcationCurrentStepNumber >= 100
        ? { width: "100%" }
        : { width: this.verifcationCurrentStepNumber + "%" };
    }
  },
  methods: {
    getProgressClassName : function(progressState) {
      if (progressState === stateProgress.NOTSTARTED) {
        return stateProgressClassName.NOTSTARTED;
      } else if (progressState === stateProgress.PROGRESSING) {
        return stateProgressClassName.PROGRESSING;
      } else if (progressState === stateProgress.DONE) {
        return stateProgressClassName.DONE;
      }      
    }
  },
  created() {}
});
@import url("https://fonts.googleapis.com/css2?family=Roboto&display=swap");
 p {
     margin: 0;
}
 #page {
     font-family: Roboto;
     width: 100%;
     background: #f8f9fa;
     display: flex;
     flex-direction: column;
     align-items: center;
}
 .container {
     width: 70%;
     text-align: center;
}
 .progress-bar {
     display: flex;
     flex-direction: row;
     align-items: flex-start;
     width: 100%;
}
 .point {
     width: 38px;
     display: flex;
     flex-direction: column;
     align-items: center;
     gap: 8px;
     color: #28a745;
}
 .step-indicator-not-started {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: none;
}
 .step-indicator-progressing {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: 4px solid #28a745;
}
 .step-indicator-done {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #28a745;
     box-sizing: border-box;
     border: 4px solid #28a745;
}
 .bar {
     width: 100%;
     height: 16px;
     display: grid;
     align-items: center;
}
 .bar-eform {
     width: 80%;
}
 .bar-verification {
     width: 20%;
}
 .bar-bg {
     height: 4px;
     background-color: #ecedee;
     position: relative;
}
 .bar-arrow {
     position: absolute;
     top: 0;
     left: 0;
     height: 4px;
     background-color: #28a745;
}
 .page-indicator {
     color: #28a745;
     font-size: 14px;
}
 
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="page">
  <div class="container">
    <h3>{{encourageText}}</h3>
    <div class="progress-bar">
      <div class="point">
        <div :class="checkEFormProgress">
        </div>
        <p><strong>Account</strong></p>
      </div>
      <div class="bar bar-eform">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="eformProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="checkVerificationProgress">
        </div>
        <p><strong>Verification</strong></p>
      </div>
      <div class="bar bar-verification">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="verificationProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="checkReviewProgress">
        </div>
        <p><strong>Review</strong></p>
      </div>
    </div>
  </div>
  <div class="page-indicator">
    <strong>{{currentStepNumber}}/{{totalStepNumber}}</strong>
  </div>
</div>

\$\endgroup\$

1 Answer 1

1
+50
\$\begingroup\$

It appears that the second version pulls the states out into an enum-like object. This does make it simpler to work with in the methods for the computed properties. Bear in mind that while stateProgress and stateProgressClassName are declared with const so they cannot be re-assigned, it is possible to assign properties on them. If that should be prevented then use Object.freeze().

The method getProgressClassName() could just verify that the value in progressState exists in stateProgress (using Object.values()) and if so, return "step-indicator-" + stateProgress. However, it appears that those calls names are redundant also. Let's look at the styles for those classes:

 .step-indicator-not-started {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: none;
}
 .step-indicator-progressing {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #ecedee;
     box-sizing: border-box;
     border: 4px solid #28a745;
}
 .step-indicator-done {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     background: #28a745;
     box-sizing: border-box;
     border: 4px solid #28a745;
}

All three of those have four styles in common: width, height, border-radius and box-sizing.

One could abstract those styles into a separate ruleset for a new class .step-indicator that has those common styles:

.step-indicator {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     box-sizing: border-box;
}
 .not-started {
     background: #ecedee;
     border: none;
}
 .progressing {
     background: #ecedee;
     border: 4px solid #28a745;
}
 .done {
     background: #28a745;
     border: 4px solid #28a745;
}

In order to use those, the step-indicator class can be separated from the three classes, which allows those associated computed properties to be eliminated, along with stateProgressClassName and the method getProgressClassName.

For instance:

<div :class="checkEFormProgress">

Can be updated to the following array syntax:

<div :class="[eFormProgress, 'step-indicator']">

Also there is some redundancy in those last two computed properties:

eformProgressWidth() {
  return this.eformCurrentStepNumber > 100
    ? { width: "100%" }
    : { width: this.eformCurrentStepNumber + "%" };
},
verificationProgressWidth() {
  return this.verifcationCurrentStepNumber >= 100
    ? { width: "100%" }
    : { width: this.verifcationCurrentStepNumber + "%" };
}

The ternary operators could be moved inside the object:

eformProgressWidth() {
  return { 
    width: this.eformCurrentStepNumber > 100
      ? "100%" 
      : this.eformCurrentStepNumber + "%" 
  };
},
verificationProgressWidth() {
  return { 
    width: this.verifcationCurrentStepNumber >= 100 
      ? "100%" 
      : this.verifcationCurrentStepNumber + "%" 
  };
}

The Math.min() function can help simplify these methods to a single line each:

    eformProgressWidth() {
      return { width: Math.min(this.eformCurrentStepNumber, 100) + "%" };
    },
    verificationProgressWidth() {
      return { width: Math.min(this.verifcationCurrentStepNumber, 100) + "%" };
    }

These suggestions can lead to greatly reduced code.

const stateProgress = {
  NOTSTARTED: "not-started",
  PROGRESSING: "progressing",
  DONE: "done"
};

new Vue({
  el: "#page",
  data: {
    currentStepNumber: 2,
    totalEformStepNumber: 9,
    totalVerificationStepNumber: 2,
    totalReviewStepNumber: 1,
    eFormProgress: stateProgress.NOTSTARTED,
    verificationProgress: stateProgress.NOTSTARTED,
    reviewProgress: stateProgress.NOTSTARTED,
    encourageText: "Let's Start"
  },
  computed: {
    totalStepNumber() {
      return (
        this.totalEformStepNumber +
        this.totalVerificationStepNumber +
        this.totalReviewStepNumber
      );
    },
    eformCurrentStepNumber() {
      if (this.currentStepNumber <= 0) {
        this.eFormProgress = stateProgress.NOTSTARTED;
      } else if (this.currentStepNumber < this.totalEformStepNumber) {
        this.eFormProgress = stateProgress.PROGRESSING;
      } else if (this.currentStepNumber >= this.totalEformStepNumber) {
        this.eFormProgress = stateProgress.DONE;
      }
      return (this.currentStepNumber / this.totalEformStepNumber) * 100;
    },
    verifcationCurrentStepNumber() {
      //debugger;
      if (this.currentStepNumber >= this.totalEformStepNumber) {
        if (this.currentStepNumber < this.totalEformStepNumber) {
          this.verificationProgress = stateProgress.NOTSTARTED;
        } else if (
          this.currentStepNumber > this.totalEformStepNumber &&
          this.currentStepNumber < this.totalStepNumber
        ) {
          this.verificationProgress = stateProgress.PROGRESSING;
        } else if (this.currentStepNumber === this.totalStepNumber) {
          this.verificationProgress = stateProgress.DONE;
          this.reviewProgress = stateProgress.PROGRESSING;
        }
        return (
          ((this.currentStepNumber - this.totalEformStepNumber) /
            this.totalVerificationStepNumber) *
          100
        );
      } 
      return 0;
    },
    eformProgressWidth() {
      return { width: Math.min(this.eformCurrentStepNumber, 100) + "%" };
    },
    verificationProgressWidth() {
      return { width: Math.min(this.verifcationCurrentStepNumber, 100) + "%" };
    }
  },
  created() {}
});
@import url("https://fonts.googleapis.com/css2?family=Roboto&display=swap");
 p {
     margin: 0;
}
 #page {
     font-family: Roboto;
     width: 100%;
     background: #f8f9fa;
     display: flex;
     flex-direction: column;
     align-items: center;
}
 .container {
     width: 70%;
     text-align: center;
}
 .progress-bar {
     display: flex;
     flex-direction: row;
     align-items: flex-start;
     width: 100%;
}
 .point {
     width: 38px;
     display: flex;
     flex-direction: column;
     align-items: center;
     gap: 8px;
     color: #28a745;
}
.step-indicator {
     width: 15px;
     height: 15px;
     border-radius: 100px;
     box-sizing: border-box;
}
 .not-started {
     background: #ecedee;
     border: none;
}
 .progressing {
     background: #ecedee;
     border: 4px solid #28a745;
}
 .done {
     background: #28a745;
     border: 4px solid #28a745;
}
 .bar {
     width: 100%;
     height: 16px;
     display: grid;
     align-items: center;
}
 .bar-eform {
     width: 80%;
}
 .bar-verification {
     width: 20%;
}
 .bar-bg {
     height: 4px;
     background-color: #ecedee;
     position: relative;
}
 .bar-arrow {
     position: absolute;
     top: 0;
     left: 0;
     height: 4px;
     background-color: #28a745;
}
 .page-indicator {
     color: #28a745;
     font-size: 14px;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="page">
  <div class="container">
    <h3>{{encourageText}}</h3>
    <div class="progress-bar">
      <div class="point">
        <div :class="[eFormProgress, 'step-indicator']">
        </div>
        <p><strong>Account</strong></p>
      </div>
      <div class="bar bar-eform">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="eformProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="[verificationProgress, 'step-indicator']">
        </div>
        <p><strong>Verification</strong></p>
      </div>
      <div class="bar bar-verification">
        <div class="bar-bg">
          <div class="bar-arrow" v-bind:style="verificationProgressWidth"></div>
        </div>
      </div>
      <div class="point">
        <div :class="[reviewProgress, 'step-indicator']">
        </div>
        <p><strong>Review</strong></p>
      </div>
    </div>
  </div>
  <div class="page-indicator">
    <strong>{{currentStepNumber}}/{{totalStepNumber}}</strong>
  </div>
</div>

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the code review. So the CSS, if I use SCSS, it will be simpler? \$\endgroup\$
    – Steve Ngai
    Feb 8, 2022 at 5:24

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.