none
Indexing array is rewriting my elements RRS feed

  • Question

  • Good evening everyone,

    There is currently an issue with my code and I think it has to do with my for loop. I think my for loop is rewriting all of my elements within my array and I cant figure out how to fix it. I have been trying to work this out for some time by reviewing older threads, but I cannot find a resolution to this issue. Here is my code.

    namespace StoreFront
    {
        public class Store
        {
            static public void Main(string[] args)
            {
                int productNum = 1;
                bool exit = false;
                while (!exit)
                {
                    string input;
                    Bill bill = new Bill();
                    Console.WriteLine("Welcome to Teenage Mutant Ninja Turtle's Pizza Shop!");
                    int attributeOne = PromptForSelection("Pizza Size", Product.ATTRIBUTE_ONE_TYPE);
                    int attributeTwo = PromptForSelection("Pizza Style", Product.ATTRIBUTE_TWO_TYPE);
                    Product[] product = new Product[productNum];
                    
                    //for(List<Product>.Enumerator
                    for (int i = 0; i < product.Length; i++)
                    {
                        product[i] = new Product(attributeOne, attributeTwo);
                        var newProduct = product[i];
                        bill.Add(newProduct);
                    }
                    DisplayProduct("Product 1", product[productNum-1]);
                    Console.WriteLine(bill.Display());
                    Console.Write("Would you like to place another order? [y/n]");
                    input = Console.ReadLine();
                    Console.WriteLine();
                    switch (input.ToLower())
                        {
                           default:
                           {
                               productNum++;
                               
                               //Product[] newProduct = new Product[productNum];
                               //newProduct[0] = new Product(attributeOne, attributeTwo);
                               //bill.Add(product[productNum]);
                           }
                           break;
                           case "n":
                           {
                               Console.WriteLine("Thank you for choosing Teenage Mutant Ninja Turtle's Pizza Shop!");
                               exit = true;
                           }
                           break;

                        }

                    }
            }

       static public int PromptForSelection(String identifier, String[] type)
       {
                Console.WriteLine("Please select a " + identifier + ": ");
       for(int index = 0; index < type.Length; index++)
       Console.WriteLine(index + ": " + type[index]);
       Console.WriteLine("Please input your selection for " + identifier + ": ");
       return int.Parse(Console.ReadLine());
       }

       static public void DisplayProduct(String identifier, Product product)
       {
        Console.WriteLine(identifier + " is a"
       + " " + Product.ATTRIBUTE_ONE_TYPE[product.AttributeOne]
       + " " + Product.ATTRIBUTE_TWO_TYPE[product.AttributeTwo]
                                + " and costs " + product.Price());
       }
        }
    }

    //////Also//////////

    namespace StoreFront
    {
        class Bill
        {
            private List<Product> order;
            //private int itemCount;

            public Bill()
            {
                this.order = new List<Product>();
            }

            public void Add(Product product)
            {
                order.Add(product);
            }

            public String Display()
            {
                string nl = Environment.NewLine;
                string receipt = String.Format("{0}This is your bill: {0}", nl);
                string description;
                double total = 0;
                foreach(Product product in order)
                { 
                    description = String.Format("{0} {1}",
                                    Product.ATTRIBUTE_ONE_TYPE[product.AttributeOne],
                                    Product.ATTRIBUTE_TWO_TYPE[product.AttributeTwo]);
                    receipt = receipt + String.Format("{1} {0}", nl, description);
                    total = total + product.Price();

                }
                receipt += String.Format("{1:C} Total for {2} items{0}", nl, total, order.Count);
                
                return receipt;
            }
        }
    }

    This is my first time posting and I am no expert. So I will be eagerly refreshing this page! Thanks for looking into this!

    Sunday, April 14, 2013 4:26 AM

Answers

  • Hi,

    For each iteration in the while loop, the whole products array is replaced with a new array that has one element more. Then the for loop sets all the elements in the new array to new Product elements created according to the current values for attributeOne and attributeTwo. (To see exactly what is happening, you could set a breakpoint and use the debugger to step through your code. Look it up!)

    Perhaps you were trying to create a new array and then use the for loop to transfer all the elements from the old array and only create a new Product in the last slot of the new array? For that to work, you would have to keep a reference to the previous array, and you would also have to make sure that only the last slot gets a new Product.

    An easier solution would be to declare products outside of the while loop, and make it a List<Product> instead of a Product[]. With a List, you can simply keep Add'ing elements and it will grow automatically. Since you don't have to keep transferring all the old items to a new Lists in each iteration, you can also get rid of the for loop.

    What is the purpose of the "products" array/list, anyway? I see that you already have a List<Product> in Bill that seems to be working as expected, so why do you need "products"?


    Marcus Björklund

    Please use "Mark As Answer" if my post has answered your question, and/or vote for it if you find it helpful. Thanks!

    • Proposed as answer by Wizend Monday, April 15, 2013 4:43 PM
    • Marked as answer by Barry WangModerator Tuesday, April 23, 2013 10:46 AM
    Monday, April 15, 2013 12:37 AM

All replies

  • Hi,

    For each iteration in the while loop, the whole products array is replaced with a new array that has one element more. Then the for loop sets all the elements in the new array to new Product elements created according to the current values for attributeOne and attributeTwo. (To see exactly what is happening, you could set a breakpoint and use the debugger to step through your code. Look it up!)

    Perhaps you were trying to create a new array and then use the for loop to transfer all the elements from the old array and only create a new Product in the last slot of the new array? For that to work, you would have to keep a reference to the previous array, and you would also have to make sure that only the last slot gets a new Product.

    An easier solution would be to declare products outside of the while loop, and make it a List<Product> instead of a Product[]. With a List, you can simply keep Add'ing elements and it will grow automatically. Since you don't have to keep transferring all the old items to a new Lists in each iteration, you can also get rid of the for loop.

    What is the purpose of the "products" array/list, anyway? I see that you already have a List<Product> in Bill that seems to be working as expected, so why do you need "products"?


    Marcus Björklund

    Please use "Mark As Answer" if my post has answered your question, and/or vote for it if you find it helpful. Thanks!

    • Proposed as answer by Wizend Monday, April 15, 2013 4:43 PM
    • Marked as answer by Barry WangModerator Tuesday, April 23, 2013 10:46 AM
    Monday, April 15, 2013 12:37 AM
  • Marcus is right. The product[] array is not only redundant, it is wrong. An array is a reference type with a fixed capacity. For every iteration of your while loop the values of the old array have to get copied to the new array with its increased capacity and the old array gets deleted. This costs a lot of performance, what could easily be avoided.

    static public void Main(string[] args) {
        bool exit = false;
        string input = String.Empty;
        Bill bill = new Bill();
        while (!exit) {
            Console.WriteLine("Welcome to Teenage Mutant Ninja Turtle's Pizza Shop!");
            int attributeOne = PromptForSelection("Pizza Size", Product.ATTRIBUTE_ONE_TYPE);
            int attributeTwo = PromptForSelection("Pizza Style", Product.ATTRIBUTE_TWO_TYPE);
            Product product = new Product(attributeOne, attributeTwo);
    
            bill.Add(product);
    
            DisplayProduct("Product 1", product);
            Console.WriteLine(bill.Display());
            Console.Write("Would you like to place another order? [y/n]");
            input = Console.ReadLine();
            Console.WriteLine();
            if (input.ToLower() == "n") { 
                Console.WriteLine("Thank you for choosing Teenage Mutant Ninja Turtle's Pizza Shop!");
                exit = true;
            }
        }
    }

    wizend

    Monday, April 15, 2013 4:45 PM