2
\$\begingroup\$

I have created this simple currency convertor to learn VueJs more. Any code review/comment on coding standard and best practice for Vue? Thanks in advance.

new Vue({
  el: "#app",
  data() {
    return {
      currency_from_amount: 0,
      currency_to_amount: 0,
      result: '',
      currencyFromOptions: [],
      currencyToOptions: [],
      loading: true,
      errored: false,
      info: null,
      currencyFromSelected: '',
      currencyToSelected: ''
    };
  },
  methods: {
    validation: function() {
      return (this.currency_from_amount > 0 && this.currencyFromSelected.length === 3 && this.currencyToSelected.length === 3)
    },
    convert: function() {
      debugger

      let payLoad = `${this.currencyFromSelected}_${this.currencyToSelected}`;
      //let payLoad = `USD_MYR`;
      console.log(payLoad);
      if(this.validation()) {
        this.loading = true;
        axios
          .get(
            `https://free.currconv.com/api/v7/convert?q=${payLoad}&compact=ultra&apiKey=afd6156ec2a14519570e`
          )
          .then((response) => {
              debugger
              let exchangeRate = JSON.stringify(response).replace(/[^0-9\.]/g, '');

              let conversionAmt = (parseFloat(this.currency_from_amount) * parseFloat(
                exchangeRate)).toFixed(2);

              this.currency_to_amount = conversionAmt;
              this.result = `${this.currency_from_amount} ${this.currencyFromSelected} equals ${conversionAmt} ${this.currencyToSelected}`;
            }

          )
          .catch(error => {
            console.log(error);
            this.errored = true;
          })
          .finally(() => (this.loading = false));
      }
    }
  },
  mounted() {
    axios
      .get("https://openexchangerates.org/api/currencies.json")
      .then(response => (this.currencyFromOptions = response.data, this.currencyToOptions = response
        .data))
      .catch(error => {
        console.log(error);
        this.errored = true;
      })
      .finally(() => (this.loading = false));
  }
});
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<!DOCTYPE html>
<!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]-->
<!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]-->
<!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]-->
<!--[if gt IE 8]>      <html class="no-js"> <!--<![endif]-->
<html>

<head>
  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <title>Currency Convertor in vue</title>
  <meta name="description" content="">
  <meta name="viewport" content="width=device-width, initial-scale=1">

  <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/css/bootstrap.min.css">
  <style>
    .form-group select {
      display: inline;
    }
  </style>
</head>

<body>
  <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
  <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/js/bootstrap.min.js"></script>
  <script src="https://cdn.jsdelivr.net/npm/vue"></script>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/axios/0.19.2/axios.min.js"></script>
  <!--[if lt IE 7]>
            <p class="browsehappy">You are using an <strong>outdated</strong> browser. Please <a href="#">upgrade your browser</a> to improve your experience.</p>
        <![endif]-->
  <div id="app">
    <div v-if="loading">Loading...</div>
    <section v-else>
      <div class="alert alert-success" id="result" v-if="validation">{{result}}</div>

      <div class="col-xs-12">
        <div class="form-group form-inline">
          <input class="form-control" type="number" name="currency_from_amount" id="currency_from_amount" v-model="currency_from_amount" @keyup="convert">

          <select class="form-control" required v-model="currencyFromSelected" @change="convert">
            <option v-for="(value,key) in currencyFromOptions" :value="key">{{ value }}</option>
          </select>
        </div>
      </div>

      <div class="col-xs-12">
        <div class="form-group form-inline">
          <input class="form-control" type="number" name="currency_to_amount" id="currency_to_amount" readonly v-model="currency_to_amount">

          <select class="form-control" v-model="currencyToSelected" @change="convert">
            <option v-for="(value,key) in currencyToOptions" :value="key">{{ value }}</option>
        </div>
      </div>
      <p>Note: Data provided by CurrencyConverterApi.com for Currency</p>
    </section>

    </select>
  </div>

</body>

</html>

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

I think that you should always use const instead of let or var if you are not going to be reassigning that variable.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ I agree, though it would help to add a reason. \$\endgroup\$ Jan 4, 2021 at 9:36
  • \$\begingroup\$ I did I said if you are not going to be reassing that variable. \$\endgroup\$ Jan 4, 2021 at 15:08
  • 1
    \$\begingroup\$ That sounds like a condition - thus it could be reworded as "if you are not going to be reassigning that variable then you should use const instead of let or var"... Imagine you are not well-versed in the differences between those keywords - one might ask "why?"... \$\endgroup\$ Jan 4, 2021 at 16:09
  • \$\begingroup\$ Thank you Sam I was not aware of this. \$\endgroup\$ Jan 4, 2021 at 16:10
  • 1
    \$\begingroup\$ The reason is to help prevent you from making mistakes in the future by accidentally overwriting a variable unexpectedly. See here: evertpot.com/javascript-let-const \$\endgroup\$
    – Caleb
    Apr 21, 2021 at 22:00
2
\$\begingroup\$

From my point of view, I think it would be better if you use same format in your data(). There you mixed between camelCase and snake_case. I don't know about your approach but make it more consistent would be better and I think it won't make you confused in the future.

currency_from_amount: 0,
currency_to_amount: 0,

to

currencyFromAmount: 0,
currencyToAmount: 0,
\$\endgroup\$

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.