3
\$\begingroup\$

I never get to use ECMA6 in production/work, so I am hoping you all can help me write better code by reviewing my finished sample:

(function() {
    'use strict';

    class BudgCalc {

        constructor(personFullName, yearlyAfterTaxIncome) {

            this.name = personFullName;

            this.income = this.dollarsToCents(yearlyAfterTaxIncome);

            this.initialAfterTaxIncome = this.income;

            this.expenses = {
                daily: [],
                weekly: [],
                monthly: [],
                yearly: []
            }

            // To be calculated at runtime
            this.incomeBreakdown = {
                _startingIncome: this.centsToDollars(this.initialAfterTaxIncome), // _ is so its at the top of the object in the console only not because its private:)
                daily: 0,
                weekly: 0,
                monthly: 0,
                yearly: 0
            }

            //  Constants storing occurances per year
            this.dateData = {
                DAILY: 365,
                WEEKLY: 52,
                MONTHLY: 12,
                YEARLY: 1
            }

            this.setAmountsAvailableByTimePeriod();

            return this;
        }

        // dollars string to cents int
        dollarsToCents(curString) {
            var dolCentArr = curString.replace('$', '').replace(',', '').split('.'),
                dollarToCent = dolCentArr[0] * 100,
                origCents = dolCentArr[1] || 0, // For exact dollar amounts
                cents;

            if (origCents.length > 2) origCents = [origCents[0], origCents[1]]; // Truncate anything larger than .00

            cents = (parseInt(dollarToCent) + parseInt(origCents)); // Avoid type coersion here!

            return Math.round(cents);
        }

        // Turns dollars int to formatted USD currency string
        centsToDollars(centsInt) {
            var centsStr = `$${(parseInt(centsInt) / 100)}`,
                formattedUSMoneyArr = centsStr.split('.'),
                centsMember = formattedUSMoneyArr[1] || [],
                result;
            if (!centsMember.length) return centsStr + '.00'; // Add back in string format when js truncates a whole number (trailing .00's)
            else switch (true) {
                case centsMember.length === 1:
                    return centsStr + '0';
                    break; // .0+0
                case centsMember.length === 2:
                    return centsStr;
                    break; // Ok format here just return
                case centsMember.length > 2:
                    return formattedUSMoneyArr[0] + '.' + centsMember[0] + centsMember[1];
                    break; // Truncate larger than 99 cents 
            }
        }

        // Sets up the amounts available in time period
        setAmountsAvailableByTimePeriod() {
            this.incomeBreakdown.daily = this.centsToDollars(this.income / this.dateData.DAILY);
            this.incomeBreakdown.weekly = this.centsToDollars(this.income / this.dateData.WEEKLY);
            this.incomeBreakdown.monthly = this.centsToDollars(this.income / this.dateData.MONTHLY);
            this.incomeBreakdown.yearly = this.centsToDollars(this.income);
            return this;
        }

        setNewExpense(name, recurance, amount) {
            var bill = {},
                rate = this.dateData[recurance.toUpperCase()],
                matchingRecord = this.expenses[recurance];

            amount = this.dollarsToCents(amount);

            if (this.incomeBreakdown[recurance] && rate && matchingRecord) {
                this.income = Math.round(this.income - (amount * rate));
                bill[name] = this.centsToDollars(amount);
                matchingRecord.push(bill);
                this.setAmountsAvailableByTimePeriod();
            }

        }

        getFullReport() {
            return {
                expenses: this.expenses,
                budget: this.incomeBreakdown
            }
        }
    }

    class RenderUI extends BudgCalc {
        addExpense(nameStr, recuranceStr, amountStr) {
            // UI stuff in here, just leaving focus on the logic for now
            this.setNewExpense(nameStr, recuranceStr, amountStr);
        }
    }

    var app = new RenderUI('John Doe', (6450 * 24).toString() + '.00');

    app.addExpense('health insurance', 'monthly', '$200.00');
    app.addExpense('dining', 'daily', '$20.00');
    app.addExpense('car payment', 'monthly', '$300.00');
    app.addExpense('car insurance payment', 'monthly', '$300.23');
    app.addExpense('rent', 'monthly', '$1650.00');
    app.addExpense('cable', 'monthly', '175.00');
    app.addExpense('food', 'monthly', '$800.00');
    app.addExpense('gasoline', 'monthly', '$400.00233');
    app.addExpense('electric', 'monthly', '$110.00');
    app.addExpense('maggies phone', 'monthly', '$120.00');
    app.addExpense('savings', 'monthly', '$820.00');

    console.log({
        incomeReport: app.getFullReport()
    });

})();

Here is a demo: http://codepen.io/nicholasabrams/pen/zqKRYd/

\$\endgroup\$
3
\$\begingroup\$

Your code looks pretty good but some suggestions are:

Use let

If you're going to use ES6, you should probably use let. let generally helps avoid problems with scoping. Try to also use const for constants.

Use static

If your function is not referencing this you should probably use static.

dollarsToCents(curString) {

to...

static dollarsToCents(curString) {

this may mean you'll have to change parts of your code but it allows access to these functions without having to go through prototype or create an instance of the class.

Try to make conditionals clear at first glance

When I see:

!centsMember.length

It takes me a moment to figure out what this means. Rewriting this makes this a lot more clear:

centsMember.length === 0

Use a radix on parseInt!

You're using parseInt without a radix. On some browsers, sometimes the browser will assume the wrong base which will cause problems, instead use:

parseInt(n, 10)

if your "stringified" number is guaranteed to be an integer, you can use any of the following:

Number(n)
+n

Try to simplify code

When I first saw this, it took me a moment to see you're trying to get the first to digits.

if (origCents.length > 2) origCents = [origCents[0], origCents[1]]

Also, note this returns an array which you are passing into parseInt which will cause problems. Instead try something like:

origCents = origCents.substr(0, 2);

Try to use regex more

Right here you have a couple of regexes:

.replace('$', '').replace(',', '')

instead you could simplify this to:

.replace(/[$,]/g, '');

this will also be global and will remove all occurrences.

String templates

You're using string templates which is awesome but you can still make some more things string templates:

(6450 * 24).toString() + '.00'

can become...

`${6450 * 24}.00`
\$\endgroup\$
  • \$\begingroup\$ Thank you that was very helpful. Just wondering, do you know if using let and const can also help performance? \$\endgroup\$ – Alpha G33k Mar 16 '16 at 1:26
  • \$\begingroup\$ @AlphaG33k The JavaScript engine will be able to optimize better when you're using let and const where applicable. While you may not notice a very significant speed boost, the engine will be optimizing. \$\endgroup\$ – Downgoat Mar 16 '16 at 1:28
  • \$\begingroup\$ Moreover on the parseInt point, you could use Number instead, which doesn't require a radix and is more clearer IMO \$\endgroup\$ – Quill Mar 16 '16 at 1:29
  • \$\begingroup\$ @Quill true but Number does not ensure an integer \$\endgroup\$ – Downgoat Mar 16 '16 at 1:30
  • \$\begingroup\$ Number is an inherited integer, they are, for all intensive purposes, identical. \$\endgroup\$ – Quill Mar 16 '16 at 1:31
2
\$\begingroup\$

Also, you should use Arrow Functions.

For eg:

centsToDollars(centsInt) {
  ...
}

would look something like:

centsToDollars = (centsInt) => {
  ...
}

As a side note, if you have a function with a single return value like:

centsToDollars(centsInt) {
  return (centsInt);
}

you could write that as:

centsToDollars = centsInt => centsInt

Hope that helps! :)

\$\endgroup\$
  • \$\begingroup\$ Yes thank you, I didn't think of this feature! \$\endgroup\$ – Alpha G33k Mar 17 '16 at 15:12
  • 1
    \$\begingroup\$ Should be noted that arrow functions in class definitions is an experimental ES7 feature - not a feature of ES6. You can't do this without a transpiler (Babel) and the appropriate configurations. \$\endgroup\$ – Oka Mar 17 '16 at 21:33
  • \$\begingroup\$ May I suggest improving your answer by explaining the difference of arrow functions (does not bind its own this, arguments, super, or new.target and are always anonymous.) \$\endgroup\$ – Seth McClaine Mar 18 '16 at 18:08
  • \$\begingroup\$ @SethMcClaine I've already attached the link in my answer and from there you will be able to know about arrow functions in details. But, anyways, thanks for the suggestion :) \$\endgroup\$ – Nirmalya Ghosh Mar 18 '16 at 18:11

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.