HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Currency converter using angular.js

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usingangularconvertercurrency

Problem

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


    
    Index
    
    

    
        Currency converter
            
        
        

        

        
        

        

        
            1 {{from.selectedcurrency.description}} equals 
            {{(convert(from.selectedcurrency.currency, to.selectedcurrency.currency, 1)|number:3)+" "+to.selectedcurrency.description}}
        
    
    
    


currencycontrol.html


    
        
    
    
        
        
    


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].val

Solution

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
under the `. is a block-level element. This means whatever comes after it is forced under it. No need for the
. If the
is for spacing, use margin or padding instead. Also, tags are for headings. They're not for sizing. Use CSS to size the elements. Seeing that they're normal text, ` should suffice.

Context

StackExchange Code Review Q#119009, answer score: 2

Revisions (0)

No revisions yet.