This is my first angular application. I am sure there must be many mistakes like coding conventions, naming conventions or implementation mistakes like what goes where. Could you please review this application which converts currency from one to another.
Index.html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Index</title>
<link rel="stylesheet" href="bootstrap.css">
<link rel="stylesheet" href="CurrencyConverter.css">
</head>
<body ng-app='myApp'>
<div ng-controller="CurrencyConverter" class="panel panel-default board">
<h3>Currency converter</h3>
<br>
<currencycontrol
currencyobject = from
currencies = currencies
direction = direction
source = 1
>
</currencycontrol>
<br>
<currencycontrol
currencyobject=to
currencies = currencies
direction = direction
source = 0
>
</currencycontrol>
<br>
<div class="row description">
<div class="col-xs-12 txt txt-large"><h5>1 {{from.selectedcurrency.description}} equals </h5></div>
<div class="col-xs-12 txt txt-large"><h3>{{(convert(from.selectedcurrency.currency, to.selectedcurrency.currency, 1)|number:3)+" "+to.selectedcurrency.description}}</h3></div>
</div>
</div>
<script src="angular.js" type="text/javascript"></script>
<script src="Index.js" type="text/javascript"></script>
</body>
</html>
currencycontrol.html
<div class='row'>
<div class='col-xs-8'>
<input type='number' placeholder='{{currencyobject.selectedcurrency.currency}}' class='form-control' ng-model='currencyobject.amount' ng-focus='changeDirection(source)'>
</div>
<div class='col-xs-4'>
<select name='currencySelect' id='currencySelect' ng-model='currencyobject.selectedcurrency'
ng-options='c.currency for c in currencies' class='form-control' ng-focus='changeDirection(1)'>
</select>
</div>
</div>
Index.js
var app = angular.module("myApp", []);
function getCurrencies()
{
var a =
[
{
currency: 'INR',
value: 1,
description: 'Indian Rupee'
},
{
currency: 'USD',
value: 0.015,
description: 'US Dollar'
},
{
currency: 'GBP',
value: 0.010,
description: 'British Pound'
},
{
currency: 'AED',
value: 0.054,
description: 'United Arab Emirates Dirham'
},
{
currency: 'AUD',
value: 0.021,
description: 'Australian Dollar'
},
{
currency: 'EUR',
value: 0.014,
description: 'Euro'
}
];
return a;
}
app.controller('CurrencyConverter', function($scope, $filter)
{
$scope.currencies = getCurrencies();
$direction = 0;
$scope.convert = function(from, to, amount)
{
var fromFound = $filter('filter')($scope.currencies, {currency: from}, true);
var toFound = $filter('filter')($scope.currencies, {currency: to}, true);
var IndianAmount = 0;
if(fromFound.length)
IndianAmount = amount/fromFound[0].value;
var toAmount = toFound[0].value*IndianAmount
return toAmount;
}
$scope.updateTarget = function()
{
$scope.to.amount =
$scope.convert(
$scope.from.selectedcurrency.currency,
$scope.to.selectedcurrency.currency,
$scope.from.amount)
if($scope.to.amount == 0)
$scope.to.amount = '';
}
$scope.updateSource = function()
{
$scope.from.amount =
$scope.convert(
$scope.to.selectedcurrency.currency,
$scope.from.selectedcurrency.currency,
$scope.to.amount)
if($scope.from.amount == 0)
$scope.from.amount = '';
}
$scope.from =
{
selectedcurrency : $scope.currencies[0]
};
$scope.to =
{
selectedcurrency : $scope.currencies[1]
};
$scope.$watch(
'from.amount',
function(newValue, oldValue)
{
if($scope.direction == 0)
$scope.updateTarget();
}
);
$scope.$watch(
function()
{
return $scope.from.selectedcurrency.currency;
},
function(newValue, oldValue)
{
if($scope.direction == 0)
$scope.updateTarget();
}
);
$scope.$watch(
function()
{
return $scope.to.selectedcurrency.currency;
},
function(newValue, oldValue)
{
if($scope.direction == 0)
$scope.updateTarget();
}
);
$scope.$watch(
function()
{
return $scope.to.amount;
},
function(newValue, oldValue)
{
if($scope.direction == 1)
$scope.updateSource();
}
);
});
app.directive('currencycontrol', function()
{
return {
scope:{
currencyobject:'=',
currencies:'=',
direction:'=',
source:'='
},
restrict: "E",
templateUrl: "currencycontrol.html",
controller: function($scope)
{
$scope.changeDirection = function(source)
{
if(source == 0)
{
$scope.direction = 1;
}
else
{
$scope.direction = 0;
}
}
}
};
});
CurrencyConverter.css
.board
{
width: 700px;
margin: 100px;
box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2), 0 6px 20px 0 rgba(0, 0, 0, 0.19);
background-color: transparent;
padding-right:40px;
padding-left:40px;
padding-top:20px;
padding-bottom:20px;
}
.description h3
{
margin:0px;
}
angular.js is of version 1.5
-
\$\begingroup\$ jsfiddle output is jsfiddle.net/nagasandeepbondili/u8jope3u/4 \$\endgroup\$Naga Sandeep– Naga Sandeep2016年02月05日 17:58:15 +00:00Commented Feb 5, 2016 at 17:58
1 Answer 1
First, variables. fromFound and toFound doesn't seem clear to me. What are they? I don't remember finding anything in the app.
Over to your numbers, you clip your the numbers below the inputs to 3 decimal places. However, the inputs don't get the same treatment.
Next is your use of $watch. AngularJS already has an internal watch system (I believe they call it "digest"). It periodically checks for changes in the data and recompute anything that changes. Usage of $watch is like putting a "watch in a watch".
Your currency control appears to do too much. It should only know about the value and currency bindings. For switching source and destination, it should accept a function from the controller which it will call when it is focused. However, all the logic for switching who is source and destination is the responsibility of the controller.
Then I see this <br> under the <h3>. <h3> is a block-level element. This means whatever comes after it is forced under it. No need for the <br>. If the <br> is for spacing, use margin or padding instead. Also, <h*> tags are for headings. They're not for sizing. Use CSS to size the elements. Seeing that they're normal text, <span> should suffice.