0
votes

My goal is to make this program to take a number of pizzas and types of pizzas and count how much they cost. I decided to go with an object solution. The problem is it doesn't calculate it and it lets the program run even when The fields are empty. I literally have no idea why it doesn't calculate it. I'm also new to objects so there may be some logical mistakes.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Assignment_2
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }
        private void OrderButton_Click(object sender, EventArgs e)
        {
            double withTax = 0;
            double tax = 0;
            double subTotal = 0;
            var pizzas = new Pizza[3];
            if(ValidateAndDeclareQuantities())
            {
                pizzas = Declare();
                subTotal = CalcSubTotal(pizzas);
                tax = CalcTax(pizzas);
                withTax = CalcWithTax(pizzas);
            }

        }
        bool ValidateAndDeclareQuantities()
        {
            var combolist = new List<ComboBox>();
            combolist.Add(comboBox1);
            combolist.Add(comboBox2);
        combolist.Add(comboBox3);
        var textboxlist = new List<TextBox>();
        textboxlist.Add(Quantity1);
        textboxlist.Add(Quantity2);
        textboxlist.Add(Quantity3);
        for (int i = 0; i < 3; i++)
        {
            if (combolist[i].Text == "Cheese" || combolist[i].Text ==     "Vegetable" || combolist[i].Text == "Meat")
            { }
            else combolist[i].Text = "Wrong input";
        }
        int[] Quantities = new int[3];
        for (int i = 0; i < 3; i++)
        {
            if (int.TryParse(textboxlist[i].Text, out     Quantities[i])&&textboxlist[i].Text!=null)
            { }
            else { textboxlist[i].Text = "Wrong input"; }
        }
        return true;
    }

    Pizza[] Declare()
    {
        var pizzas = new Pizza[3];
        string type;
        int price;
        type = comboBox1.Text;
        price = int.Parse(Quantity1.Text);
        Pizza pizza1 = new Pizza(type, price);
        pizzas[0] = pizza1;

        type = comboBox2.Text;
        price = int.Parse(Quantity2.Text);
        Pizza pizza2 = new Pizza(type, price);
        pizzas[1] = pizza2;

        type = comboBox3.Text;
        price = int.Parse(Quantity3.Text);
        Pizza pizza3 = new Pizza(type, price);
        pizzas[2] = pizza3;

        return pizzas;
    }

    double CalcSubTotal(Pizza[] pizzas)
    {
        double subTotal = 0;
        for (int i = 0; i < 3; i++)
        {
            subTotal += pizzas[i].Price;
        }
        return subTotal;
    }

    double CalcTax(Pizza[] pizzas)
    {
        double tax = 0;
        for (int i = 0; i < 3; i++)
        {
            tax += pizzas[i].Tax;
        }
        return tax;
    }

    double CalcWithTax(Pizza[] pizzas)
    {
        double withTax = 0;
        for (int i = 0; i < 3; i++)
        {
            withTax += pizzas[i].WithTax;
        }
        return withTax;
    }

    void WriteOut(double subTotal, double tax, double withTax)
    {
        lblSubTotal.Text = "" + subTotal;
        lblTax.Text = "" + tax;
        lblTotal.Text = "" + withTax;
    }

    }
}

And the class: using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Threading.Tasks;

namespace Assignment_2
{
class Pizza
{
    string type;
    int quantity;
    public double Price;
    public double SubTotal;
    public double Tax;
    public double WithTax;
    public Pizza(string type, int quantity)
    {
        this.type = type;
        this.quantity = quantity;
        FindPrice();
        CalcSubTotal();
        CalcTax();
        CalcWithTax();
    }
    private void FindPrice()
    {
        switch (type)
        {
            case "Cheese":
                Price = 9.95;
                break;
            case "Vegetables":
                Price = 10.95;
                break;
            case "Meat":
                Price = 11.95;
                break;
        }
    }
    private void CalcSubTotal()
    {
        SubTotal = Price * quantity;
    }
    private void CalcTax()
    {
        Tax = SubTotal * 0.13;
    }
    private void CalcWithTax()
    {
       WithTax = SubTotal + Tax;
    }
}
}

Solution form

1
Sorry to say .. but you need to rethink the whole setup here. At least the while validate - declare partJim

1 Answers

2
votes

The quick answers:

  • ValidateAndDeclareQuantities never returns false. It should (probably) return false when you set "Wrong Input".

  • (Minor) int[] Quantities = new int[3]; is never used, aside from writing to it.

  • (Minor) var pizzas = new Pizza[3]; is also never used. It just gets overwritten by Declare a few lines later. Pizza[] pizzas=null; or just Pizza[] pizzas; is a better alternative. Not the greatest structure here though.

  • (Minor) Your variable called price in Declare is poorly named as it appears to actually be quantity. Things like this easily throw people off.

  • WriteOut is never called. withTax, tax and subTotal in OrderButton_Click are probably being computed correctly, but the values aren't being outputted.

The longer answer

It's a bit on the messy side! I appreciate that it's just a learning thing - we've all been there - but good code hygiene is just as important (if not more important) than the structure of the language.

UX: Don't overwrite what the user entered - specifically, don't replace the textbox input with "wrong input"; That's better off going on some other label. I would imagine you've already felt how weird this kind of experience is whilst testing the code.

Named things that don't need a specific class: Like a cheese pizza and a ham one. Enums are your friend! Use them instead of strings like "Cheese":

public enum PizzaType{
    Cheese,
    Tomato
}

Using enums in this way helps avoid the wonderful world of pain that is unexpected capitalisation and it's considerably faster too. CheEse pizza anyone?

Repetition: Large portions of your code are repetitive too; You'll want to practice avoiding it as much as you can. ('DRY'/ 'Don't Repeat Yourself'). A little forward planning helps massively. Everybody has preferences on code structure; mine here would be a separate "Pizza displayer" class which holds a quantity input box and does the validation too.

Junk: Slightly related to the above, you're creating a bunch of Lists and arrays which get created each time the function is called and then are just chucked out. Create a single array of some more abstract type (like an array of "Pizza displayers") and keep that array as a property on the Form. It's minor here, but being more aware of how much trash your program creates helps make your code go faster.

Notes on floats: You should never, ever use float/ double for money. Use decimal instead, or just do everything in pennies. Floating points aren't precise and you'll hit a rounding issue sooner or later.