Currency converter built with AngularJS

Question

I am fairly new to AngularJS and I am trying to improve my knowledge of this JavaScript Framework by building small real-world web-apps.

In this case, I have built a currency converter app with harnesses an exchange rate API built by me which runs here.

I want to know whether there exists a scope of improvement and optimization in my code.

app.js

var app = angular.module('QConvert', ['qconvertctrlmodule']);

qconvertctrl.js

var app = angular.module('qconvertctrlmodule', [])
 .controller('QConvertController', function($scope, $http, $log, $interval) {
 $scope.currencyObject = {
 from : "",
 to : "",
 amount : "",
 amount_converted : "",
 exchangeRate : ""
 };
 $scope.currencyCodes = [{value : 'INR', display : 'Indian Rupee'}, {value : 'USD', display : 'US Dollar'}, {value : 'GBP', display : 'British Pound'}];
 $scope.getexchangerate = function() {
 $log.info("FROM : " + $scope.currencyObject.from);
 $log.info("TO : " + $scope.currencyObject.to);
 $http.get("http://api.decoded.cf/currency.php", {params : {from : $scope.currencyObject.from,
 to : $scope.currencyObject.to, amount : 1}})
 .then(function(response) {
 $scope.currencyObject.exchangeRate = response.data.amount_converted;
 $log.info(response.data.amount_converted);
 });
 };
 $interval(function() {
 $scope.getexchangerate();
 console.log('Exchange rates refreshed!');
 }, 5000);
 });
app.filter('toDecimal', function() {
 return function(input, precision) {
 return input.toFixed(precision);
 };
});

index.html

<div class="container">
 <div class="page-header">
 <h1>QConvert</h1>
 </div>
 <p class="lead">A Seamless Currency Converter</p>
 <div class="row" ng-controller="QConvertController">
 <div class="col-md-8 col-md-offset-2">
 <div class="form-group">
 <label for="amount">Amount</label>
 <input type="number" step="any" class="form-control" id="amount" ng-model="currencyObject.amount">
 </div>
 </div>
 <div class="col-md-8 col-md-offset-2">
 <div class="form-group">
 <label for="from">From</label>
 <select class="form-control" id="from" ng-model="currencyObject.from" ng-change="getexchangerate()">
 <option ng-repeat="currencyCode in currencyCodes" value="{{currencyCode.value}}">{{currencyCode.display}}</option>
 </select>
 </div>
 </div>
 <div class="col-md-8 col-md-offset-2">
 <div class="form-group">
 <label for="to">To</label>
 <select class="form-control" id="to" ng-model="currencyObject.to" ng-change="getexchangerate()">
 <option ng-repeat="currencyCode in currencyCodes" value="{{currencyCode.value}}">{{currencyCode.display}}</option>
 </select>
 </div>
 </div>
 <br>
 <br>
 <br>
 <div class="col-md-8 col-md-offset-2" ng-show="currencyObject.amount">
 <h3 class="display-4" ng-If="currencyObject.from != currencyObject.to">{{currencyObject.amount}} {{currencyObject.from}} <span ng-show="currencyObject.to">=</span> <span ng-show="(currencyObject.amount * currencyObject.exchangeRate)">{{(currencyObject.amount * currencyObject.exchangeRate) | toDecimal:3}}</span> {{currencyObject.to}}</h3>
 <h3 class="display-4" ng-If="(currencyObject.from == currencyObject.to && currencyObject.from !== undefined && currencyObject.to !== undefined)">{{currencyObject.amount}} {{currencyObject.from}} = {{currencyObject.amount}} {{currencyObject.from}}</h3>
 </div>
 </div>
 </div>

The app is running live here.

• Dan: Consider using the currency filter to display the numbers with currency symbols as prefixes.
• aaronmallen: the only input I really have is you could use the angular $number filter as opposed to this custom one you've built.

Answer 1

Feedback #1:

Angular can actually build the select for you without the repeat via the ng-options attribute directive:

<select class="form-control" 
 id="to" 
 ng-model="currencyObject.to" 
 ng-change="getexchangerate()" 
 ng-options="currencyCode.value as currencyCode.display for currencyCode in currencyCodes"></select>

Feedback #2:

Angular provides a filter for formatting numbers, $number, so no need to write your own.

Feedback #3:

Use proper camelCase for function names (i.e. getExchangeRate instead of getexchangerate). As well as don't mix your snake_case and camelCase. Use snake_case for CONSTANTS (of which you have none) (i.e. $scope.currencyObject.amountConverted instead of $scope.currencyObject.amount_converted).

Feedback #4:

Remove unused $scope properties. You never set nor call $scope.currencyObject.amount_converted, so get rid of it.

Feedback #5:

You're making a request without having all of your params. If I change the value of the first select, but haven't changed the value of the second you are firing the promise each time. This can easily be fixed with a simple if statement:

// Set initial values null, so they return false.
$scope.currencyObject = {
 from : null,
 to : null,
 amount : 0,
 exchangeRate : null
}
$scope.getExchangeRate = function() {
 $log.info("FROM : " + $scope.currencyObject.from);
 $log.info("TO : " + $scope.currencyObject.to);
 var options = {
 url: 'http://api.decoded.cf/currency.php',
 method: 'GET',
 params: {
 from : $scope.currencyObject.from,
 to : $scope.currencyObject.to, 
 amount : $scope.currencyObject.amount
 }
 };
 if ($scope.currencyObject.from && $scope.currencyObject.to) {
 $http(options).then(function(response) {
 $scope.currencyObject.exchangeRate = response.data.amount_converted;
 $log.info(response.data.amount_converted);
 });
 } 
};

Feedback #6:

You have no error handling around your $http request, but on an app this simplistic that may be by design.