1

Here is what I'm trying to do. I'm trying to pass an instance of order to bill, where it would be indexed. The thing is that it's not working.

Am I stretching JS too thin here?

Any example on how to do this, or some reading material?

EDIT: Maybe I should add that this is supposed to be the user interface for a POS (Point of Sale) system. It should accept the order of products (each one with variable quantity), and process in the client's side the subtotal, total and number of items in the bill.

EDIT2: Not native english speaker. Maybe the names that I choose did not best suited the problem.

function Bill (prefix,maxForms,minForms) {         
     this.prefix = prefix;
     this.maxForms = maxForms;
     this.minForms = minForms;
     this.items = [];
     this.total = 0;

     this.addOrder = function(order) {
         if (this.items.length == 0)
         {
             this.items.push(order);
         }
         for (i=0;i<this.items.length;i++){
             if (this.items[i].name === order.name) {
                 this.items[i].quantity = order.quantity;
                 this.items[i].price = order.price;
             }
             else {
                 this.items.push(order);
             }
             this.total = this.total + order.getSubTotal();
         }
     }
 }



 function Order (name,price,quantity) {

     this.name = name;
     this.price = price;
     this.quantity = quantity;

     this.getSubtotal = function () {
         return this.price*this.quantity;
     }
     this.changeQuantity = function (newQuantity) {
         this.quantity = newQuantity;
     }
     this.incrementQuantity = function () {
         this.quantity = this.quantity + 1;
     }
 }
2
  • You can pass it as json, then just decode it on server side Commented Sep 26, 2014 at 1:01
  • So how are you trying to pass it? The counter i should be kept local: for (var i=0, ...). It seems those methods should be on the constructor's prototype, not the instances. Commented Sep 26, 2014 at 1:05

4 Answers 4

1

Here's an issue:

for (i = 0;/*...*/)

I would suggest you spend a little more time in JS.
It does look a lot like C / Java / C# / PHP, etc...

The problem, however, is that JS does not have any notion of block scope*.

* until ES6, that is

It only deals with function scope.

That is, a variable has the same reference through the whole function where it's defined (via var).

If a variable is not defined via var, the function goes up to its parent to find the value of the variable, and up from there, and up from there, until it hits window.<varname>.

You might actually be modifying window.i in your class' instance.

function Bill ( ) {
    var bill = this,
        i = 0;

    for (i=0; /* ... */) { /*...*/ }
}

That said, you might do to spend time getting to know JS.
Most of what you've written looks absolutely fine, in English, as well.

I might break it down a little further:

function Bill () {
    var bill = this;
    extend(bill, {
        total : 0,
        items : [],
        addOrder : function (order) {
            var match = bill.findOrder(order.name);
            if (!match) { bill.items.push(order);  }
            else { bill.updateOrder(match, order); }
            bill.updateTotal();
        },
        findOrder : function (name) {
            var matches = bill.items.filter(function (order) {
                return order.name === name;
            });
            return matches[0];
        },
        updateOrder : function (current, updated) {
            /* I don't know if you want to replace the old order, or add to it...  */
            /* so I'm "replacing" it, instead of increasing quantity, like you did */
            current.quantity = updated.quantity;
            current.price    = updated.price;
        },
        updateTotal : function () {
            bill.total = bill.items
                .map(function (order) { return order.getSubtotal(); })
                .reduce(function (tally, price) { return tally + price; }, 0);
        }
    });
}

var bill = new Bill();
bill.addOrder(new Order(/*...*/));

I'm doing a few things differently, here.
First, extend isn't a "built-in" function; there are a lot of implementations, in all sorts of libraries, but basically, it just saves me from writing bill.x = x; bill.y = y; bill.z = z;..., and use an object, instead.

Next, I'm using var bill = this;
and bill.method = function () { bill.total = /*...*/; };
instead of this.method = function () { };, because once you go two levels down, in functions, this no longer means the object you think it does.

this.method = function () {
    this.async(function (response) {
        // unless you change it yourself, `this` probably means `window`
        this.value = response; // oops
    });
};

// instead, try
var thing = this;
thing.method = function () {
    thing.async(function (response) {
        thing.value = response;
    });
};

Of course, you can always mix and match, as long as you know how far down you can go (one level)...
...but that means you really, really need to care about using this a whole lot.

var thing = this;
this.method = function () {
    this.async(function (val) {
        thing.value = val;
    });
};

Much more confusing than just referring to the instance by a variable, rather than combining the two.

There are dozens of ways of doing this; some look very class-like, others might be 100% functional, and in ES6, you might just use classes altogether.

But there are some ideas, and some reasons behind doing them that way (especially if you don't know where the differences are in JS vs the other C-looking languages).

I don't think you're stretching JS too thin, at all.

Sign up to request clarification or add additional context in comments.

Comments

1

Once all of the issues on line 80 are fixed. All you need to do is:

var order = new Order("My Order", 12, 2);

var bill = new Bill(blah, blah, blah);

bill.addOrder(order);

Comments

0

A few issues right off the bat:

  1. this.total = this.total + order.subTotal();·
    • There is a garbage char at the end.
    • Order does not have a subtotal function. It should be getSubtotal.
  2. The 2 assignments to this.items[i].quantity and this.items[i].price are superfluous, since you are assigning properties to themselves. Remember, this.items[i] === order. This is not a bug, but it is inefficient.
  3. You should have something like this.total = 0; at the top of Bill.

1 Comment

1. Thanks. Fixed. 2. Another fix. It meant to check if the order in the array has the same name as the new order being added. If so, update it's quantities.
0

I think you want:

 this.items[i].quantity += order.quantity;
 this.items[i].price += order.price;

This will update quantity with whatever quantity order has. Secondly, I see you have an order function. Not an order object. Was that intentional? Are you planning to add instances of this bill/order object to each other? I don't think that's where you were going. Make sure they are separate objects that you are nesting.

Are you getting anything except undefined? I don't think you are because you're not returning anything. Put:

return this;

at the end of your functions. Make sure you save them to a var when you make them:

bill = Bill(v,v,v);
order = Order(v,v,v);

then you can:

bill.addOrder(order);

See if that helps.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.