3

In Visual Studio, using C#, I've got three arrays that look something like this:

int[] price = new int[] { 100, 200, 300, 400, 500, 600 };

string[] goods = new string[] { "item1", "item2", "item3", "item4", "item5", "item6" }; 

bool[] cart = new bool[] { false, false, false, false, false, false };

The boolean cart represents checkboxes. Now what I want to do is, using a button, to check what boxes are ticked, and then sort of grab the name of the item from the array goods, and the price of it from the array price. Then I'll concatenate the strings, and add up the total sum, and just display that in a MessageBox.Show.

I'm thinking I need to tie the index of the cart array somehow to the corresponding indexes of the price and goods arrays. Or is there a better way of doing this? A for loop probably, but how? I'm not sure where to start. Help greatly appreciated!

Liam
  • 27,717
  • 28
  • 128
  • 190
  • 6
    The better way is to create a separate type (class) with properties Price, Name and IsChecked or something. They you will have only one collection and can sum the prices of all objects that have `IsChecked == true` – icebat Nov 16 '18 at 15:08
  • you can make a dictionary of mapping containing `Dictionary("item1", 100)` – Kunal Mukherjee Nov 16 '18 at 15:08
  • 2
    There is better way. Create class to store your data model and then use collection to store a list of your data models. This is called OOP – Renatas M. Nov 16 '18 at 15:08
  • Where does all this data come from? *is there a better way of doing this* almost certainly yes but it's unclear what that way would be without more information – Liam Nov 16 '18 at 15:11
  • Can you be more specific about "concatenate the strings"? What kind of string result are you expecting? – Joel Coehoorn Nov 16 '18 at 15:13
  • What you're trying to do can technically work, but it's not a good path to go down. What happens is that now you've got all of these arrays they have to stay exactly in sync with each other. If you want to change one then you have to change them all. Instead, as suggested, create a single class that contains the properties of an item in a cart. Your cart can contain a collection if items. – Scott Hannen Nov 16 '18 at 15:14

6 Answers6

3

is there a better way of doing this?

Matched array like this are an anti-pattern, and a sign you should really create a class with properties for price, good, and cart. Then you can have a single array that you can loop through just once.

But you can still do what you need, even with this anti-pattern:

var items = cart.Zip(goods, (c,g) => new {Cart = c, Good = g}).Zip(price, (a, p) => new {Cart = a.Cart, Good = a.Good, Price = p});
var ticked = items.Where(i => i.Cart);

var message = string.Join(",", ticked.Select(t => t.Good));
var sum = ticked.Select(t => t.Price).Sum();

MessageBox.Show($"{message}\nTotal: {sum}");

This would also work, and might mean less memory use:

var ticked = cart.Select((value,index) => new {Cart = value, Price = price[index], Good = goods[index]})
    .Where(i => i.Cart);
var message = string.Join(",", ticked.Select(s => s.Good));
var sum = ticked.Select(s => s.Price).Sum();

MessageBox.Show($"{message}\nTotal: {sum}"); 

You can also use indexes:

int sum = 0;
var message = new StringBuilder();
var delimiter = "";
for(int i = 0; i<cart.Length; i++)
{
    if (cart[i])
    {
       sum += price[i];
       message.Append(delimiter).Append(goods[i]);
       delimiter = ",";
    }
}
MessageBox.Show($"{message}\nTotal: {sum}");

And in this case the index option is probably faster and no more difficult to understand the the linq option (though it's very often the other way around), even though it's more code.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
1

This is a simple example of a binding problem.

I assume you have a checkbox for each of your items and when it's state changes the bool[] cart changes at the correct index.

Then you just need to preserve that index and add up the right price and item like this:

string shoppingCart += goods[i];

decimal cost += price [i];

But, and that is a big one, you shouldn't rely on so many arrays based on the index.

As icebat mentioned in his comment you should implement a class representing your items.

Example:

public class Item
{
    public int Id { get; set; }
    public decimal Price { get; set; }
    public string Name { get; set; }
}
Sebastian L
  • 838
  • 9
  • 29
1

One thing you can try is making a class Good with properties Name, Price, Cart, etc

public class Good
{
    public string Name { get; set; }
    public int Price { get; set; }
    public bool Cart { get; set;}

 }

and then putting them in a list.

List<Good> MyGoods = new List<Good>();
MyGoods.Add(new Good{ Name="item1", Price=100, Cart = false});

You can add as many Items as you want to that list and then iterate them with an index or with foreach

for(int i=0; i<MyGoods.Count;i++)
{
    Console.WriteLine(MyGoods[i].Name);
}

or

foreach(Good g in MyGoods)
{
    Console.WriteLine(g.Name);
}
awful
  • 131
  • 4
0

You should definitely create an object that describes the items on sale.

however if you want to hack a horror together you can do something like this...

int[] price = new int[] { 100, 200, 300, 400, 500, 600 };

string[] goods = new string[] { "item1", "item2", "item3", "item4", "item5", "item6" }; 

bool[] cart = new bool[] { false, false, true, false, false, true };

var selected = cart.Select((value,index) => new {value, index})
    .Where(item => item.value == true)
    .Select(item => item.index)
    .ToList();

int total = 0;

foreach(var x in selected){
    total += price[x];
}

//total is 900
Loofer
  • 6,841
  • 9
  • 61
  • 102
0

As suggested by others using a data model or a dictionary would be definitely better but if you want to stick to your original code of using arrays then you can use the following code :

int[] price = new int[] { 100, 200, 300, 400, 500, 600 };

string[] goods = new string[] { "item1", "item2", "item3", "item4", "item5", "item6" };

bool[] cart = new bool[] { false, false, false, false, false, false };

StringBuilder sb = new StringBuilder();
int totalPrice = 0;
for(int i = 0; i < cart.Length; i++)
{
    if (!cart[i]) continue;
    sb.Append(goods[i]);
    totalPrice += price[i];
}
//Use sb.ToString() and totalPrice to show in Messagebox.
displayName
  • 13,888
  • 8
  • 60
  • 75
0

Though coding in C#, you are not object-oriented.

I agree with @JoelCoehoorn's comment about your current code demonstrating an anti-pattern. Below is my solution to that using Object Orientation.


  • Since price and good are referring to the same object, create an Item class and add the members String Name; and double Price; to it.

  • As there is a Cart, I'm sure the cart is in a store. So, create the Store class as well.

    • In the Store class, add a Dictionary<int, Item> AvailableItems; that lists all the items available in the store (You can, instead of a Dictionary, use some other DS as well which makes sense in your case).
  • Next, the customers who are coming to your store for purchasing anything should have a unique id assigned to them. This calls for a Customer class. (Guest users can be assigned a new random id).

    • The customers would also have a Dictionary<int, int> Cart; which is a mapping of the id of the item they want to purchase and the count of the item.
  • Lastly, we come to the Order object that takes in a Cart and has a nice method called PrintTotalSumFrom(Cart c) which does exactly what you think it should.


A useful answer I wrote some time back about Object Oriented principles is here.

displayName
  • 13,888
  • 8
  • 60
  • 75