Optimal way of approaching and coding multiple classes

Question

I have multiple classes that interact with each other and I got them to work and its diplaying properly but I cant help but feel that theres an optimal, cleaner and maybe even simpler way of coding this. Function is simple that it displays Product Name, Duration till Sold and Cost.

public class Item
    {    
    private string name;
    private int duration;
    private int cost

    public Item(string n, int d, double c)
            {
                Name = n;
                Duration = d;
                Cost = c;
            }
    }

Second class:

class Inventory
{

   private List<Item> item = new List<item>();

   public void AddItem(Item p) // Add item
            {
                item.Add(p);
            }

   public Item this[int i]
        {
            get
            {
               return item[i];
            }
            set
            {
               item[i] = value;
            }
        }
}

In my Form.cs I got this:

private void Form1_Load(object sender, EventArgs e)
{
            {
                var myInventory = new Inventory();
                var i1 = new Item("iPod", 200, 9);
                var i2 = new Item("Samsung", 700, 5);
                var i3 = new Item("Nokia", 100, 17);
                var i4 = new Item("Motorolla", 50, 50);

                myInventory.AddItem(i1);
                myInventory.AddItem(i2);
                myInventory.AddItem(i3);
                myInventory.AddItem(i4);
                lstProduct.Items.Add(myInventory[0]); // Add items into listbox via indexing
                lstProduct.Items.Add(myInventory[1]);
                lstProduct.Items.Add(myInventory[2]);
                lstProduct.Items.Add(myInventory[3]);

            }

Completely new to programming so Im still learning. Im mainly concerned with my Form.cs and the way I coded it but I welcome any feedback/suggestions if you have a better idea on how to code the other classes I made.

Thanks !

`public Item(string n, double a, int d)` That's just plain terrible. How would anyone using this code should know what does `n`, `a` and `d` stand for? — Zohar Peled, May 15 '18 at 04:02
It's great you want to learn how to write better code. However, this question is too broad for Stackoverflow. Questions about code design usually belong to [softwareengineering](https://softwareengineering.stackexchange.com). — Zohar Peled, May 15 '18 at 04:08
My mistake, Im not sure why I've put n, a and d but I fixed it to match up with the variables first letters so its easier to comprehend — Gegee, May 15 '18 at 04:10
No, don't use single letters as argument names. Use meaningful names. — Zohar Peled, May 15 '18 at 04:15
I see, Ive always thought using initials makes the code look simpler but I'll keep this in mind. thanks — Gegee, May 15 '18 at 04:23
You can also do this: `class Inventory : List { }`. That's an easy way to make a hard-coded list. — Enigmativity, May 15 '18 at 04:44
@Enigmativity I take it you don't agree with Mr. Lippert on [this one...](https://stackoverflow.com/questions/21692193/why-not-inherit-from-listt) — Zohar Peled, May 15 '18 at 04:52
@ZoharPeled - No, I agree wholeheartedly. I'm just not sure if the OP is building a _mechanism_ or a _business object_? — Enigmativity, May 15 '18 at 05:07

score 1 · Answer 1 · answered May 15 '18 at 04:19

Here's your code

using System;
using System.Collections.Generic;

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            var myInventory = new Inventory();

            var i1 = new Item{name = "iPod", cost = 200,duration = 9};
            var i2 = new Item{name = "Samsung",cost = 700,duration = 5};
            var i3 = new Item{name = "Nokia", cost = 100, duration = 17};
            var i4 = new Item{name = "Motorolla",cost = 50, duration = 50};

            myInventory.InventoryItems.Add(i1);
            myInventory.InventoryItems.Add(i2);
            myInventory.InventoryItems.Add(i3);
            myInventory.InventoryItems.Add(i4);
            foreach (var inventoryItem in myInventory.InventoryItems)
            {
                lstProduct.Items.Add(inventoryItem)
            }
        }
    }
}



public class Item
{
    public string name { get; set; }
    public int duration { get; set; }
    public int cost { get; set; }

}

class Inventory
{
    public List<Item> InventoryItems { get; set; }
}

score 0 · Answer 2 · answered May 15 '18 at 04:22

Use public (auto) property, instead of just private field, if you'd like to access it later.

// for class Item
public string Name { get; private set; }
public int Duration { get; private set; }
public int Cost { get; private set; }

// for class Inventory
public List<Item> Items { get; private set; } = new List<Item>();

Don't abbreviate variable name. Make it easy to read.

For plural items, end your variable name with "s"

Override ToString() so that you can display the object instance easily.

// For Class Item
public override string ToString()
{
   return $"Name={Name}, Duration={Duration}, Cost={Cost}";
}

// For Inventory Item
public override string ToString()
{
    return string.Join(Environment.NewLine, Items);
}

Use loop to iterate

// in Load event handler
foreach(var item in myInventory.Items)
{
    lstProduct.Items.Add(item);
}

Optimal way of approaching and coding multiple classes

2 Answers2