1

Problem: I am trying to update a List. If a certain item's ID already exists in the List, I want to add onto that item's quantity. If not, then I want to add another item to the list.

            cart = (List<OrderItem>)Session["cart"];

            for(int counter = cart.Count-1; counter >= 0; counter--)
            {
                if (cart[counter].productId == item.productId)
                {
                    cart[counter].productQuantity += item.productQuantity;
                }
                else if (counter == 0)
                {
                    cart.Add(item);
                }
            }

cart[counter] and item represent an instance(s) of a custom object of mine. Currently when I finally find a matching ID, everything APPEARS as though it should work, but I get a StackOverflowException thrown in my custom object class.

    public int productQuantity
    {
        get
        {
            return _productQuantity;
        }
        set
        {
            productQuantity = value;
        }
    }

It gets thrown right at the open-bracket of the "set". Could somebody please tell me what the heck is wrong because I've been going at this for the past 2+ hours to no avail. Thank you in advance.

4
  • 1
    On a side note, a dictionary or hashset may work better for you; no collection traversal would be necessary in that case. Commented Jun 1, 2010 at 4:35
  • Is there a reason that you return _productQuantity and set productQuantity? I usually use the same backing member for that. Commented Jun 1, 2010 at 4:38
  • @Rob: I thought it was a naming convention/data-hiding thing. I don't remember when/where I first saw it but I'd been coding my school projects like that and up until now I've never encountered a problem. I come from a Java background so the set/get properties in C# -- in Java I'd actually have to code getter and setter methods for that functionality. I think I probably just saw some code snippets somewhere before and got things mixed-up in my head. Commented Jun 1, 2010 at 5:07
  • 1
    You should use Pascal case (ProductQuantity) for your properties rather than Camel case (productQuantity) - use Camel case for your instance fields, it might make problems like these easier to spot for you. Commented Jun 1, 2010 at 5:25

4 Answers 4

8

the problem is in your setter of the productQuantity

it should read:

set
    {
        _productQuantity= value;
    }

edit (naming convention):

public class Vertex3d
{
    //fields are all declared private, which is a good practice in general 
    private int _x; 

    //The properties are declared public, but could also be private, protected, or protected internal, as desired.
    public int X
    { 
        get { return _x; } 
        set { _x = value; } 
    }
}
Sign up to request clarification or add additional context in comments.

5 Comments

on a side note: I didn't look for anyother bugs in your code posted above, just this particular one.
+1 1,000 times if I could; subtle bugs like that are one reason for paired-programming (or at least having fresh eyes stare at your code).
Dear God good-call. I didn't expect to get such a quick response either. THANKYOUTHANKYOU! I can't remember where I learned to code my properties but for some reason in the sets I never named the variable the same as I did in the gets. I thought it was a naming-convention as well as data-hiding thing... So another question: What's the proper naming convention for properties??
@KSwift: not sure how to format the comment box here sry about that. An example with few comments follows: public class Vertex3d{ private int _x; //fields are all declared private, which is a good practice in general //The properties are declared public, but could also be private, protected, or //protected internal, as desired. public int X{ get { return _x; } set { _x = value; } } } ..this is a very general answer but hopefully something useful :)
Right on. Thank you VoodooChild. I talked with some coworkers where I'm interning at and they made it clear to me exactly what happened. I gotta make sure I'm actually getting/setting the PRIVATE variable in the public property and not getting/setting the public property like I was doing in the set.
3

Replace productQuantity = value; with _productQuantity = value; (you're recurring infinitely by calling the setter over and over)

1 Comment

Thank you for the response as well but VooDoo child was first. I'm still giving you a +1 though. :-)
3

Why not just use this instead? public int productQuantity { get; set; }

But the flaw was in the _

public int productQuantity {
    get {
        return _productQuantity;
    }
    set {
        _productQuantity = value;
    }
}

cart = (List<OrderItem>)Session["cart"];
int index = cart.Find(OrderItem => OrderItem.productId == item.productId);
if(index == -1) {
    cart.Add(item);
} else {
    cart[index].productQuantity += item.productQuantity;
}

2 Comments

Thank you for the response as well but VooDoo child was first. I'm still giving you a +1 though. :-) As for your suggestion... I've never been formally taught about Lambda expressions, and although I've seen them before I've never really understood what they're for/how they operate.
you should definitely read a tutorial about them. They are quite easy to follow (basic ones anyways) and very handy
2
public int productQuantity
{
   get
   {
      return _productQuantity;
   }
   set
   {
      _productQuantity = value; //this should be an assignment to a member variable.
   }
}

1 Comment

Thank you for the response as well but VooDoo child was first. I'm still giving you a +1 though. :-)

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.