オブジェクトの検証を計算するC＃

Question

私の目標は、ピザの数と種類を取って、どれくらいの費用がかかるかを計算することです。私はオブジェクトソリューションと一緒に行くことに決めました。問題は、それが計算されず、フィールドが空であってもプログラムを実行できることです。私は文字通りそれがなぜそれを計算しないのか分かりません。私はオブジェクトにも新しいので、いくつかの論理的な間違いがあるかもしれません。

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; 
    } 

    } 
} 

とクラス： システムを使用しました。 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

Answer 1

迅速な解答：

• ValidateAndDeclareQuantitiesfalseを返すことはありません。あなたは "間違った入力"を設定すると、（おそらく）falseを返すべきです。

• （マイナー）int [] Quantities = new int [3];それを書いているのとは別に使用されることはありません。

• （マイナー）var pizzas = new Pizza[3];も使用されません。それは、数行後に宣言することによって上書きされます。 Pizza[] pizzas=null;またはちょうどPizza[] pizzas;が良い選択肢です。しかし、ここでは最大の構造ではありません。

• （マイナー）priceをDeclareに指定した変数の名前が実際の数量に見合うものではありません。このようなものは簡単に人々を捨てる。

• WriteOutは呼び出されません。 withTax,taxおよびsubTotalがOrderButton_Clickの場合はとなりますが、おそらくは正しく計算されていますが、値が出力されていません。

もはやそれは散らかっ側のビットです

に答えます！私はそれがちょうど学習のことだと思っています。私たちはそこに行ってきましたが、良いコード衛生は言語の構造と同じくらい重要です。

UX：ユーザーが入力した内容を上書きしないでください。具体的には、テキストボックス入力を「間違った入力」に置き換えないでください。他のレーベルに行く方が良いでしょう。私はあなたがすでにコードをテストしている間にこのような経験がどれほど奇妙であるかを感じたと思います。

特定のクラスを必要としない名前のついたもの：チーズのピザとハムのようなもの。 Enumsはあなたの友人です！ 「チーズ」のような文字列の代わりにそれらを使用してください：

public enum PizzaType{ 
    Cheese, 
    Tomato 
} 

このように列挙型を使用すると、予期しない時価総額で痛みの素晴らしい世界を回避するのに役立ちますし、それはあまりにもかなり高速です。 Cheeseピザの誰ですか？

反復：コードの大部分も繰り返しです。できるだけ避けて練習したいと思うでしょう。 （ 'DRY'/'Do not Repeat Yourself'）。少し前向きな計画は大規模に役立ちます。誰もがコード構造を好みます。ここでは、数量入力ボックスを保持し、検証も行う別の「Pizza displayer」クラスになります。

迷惑メール：上記と少し関係していますが、関数が呼び出されるたびに作成されたばかりのリストと配列の束を作成しているだけです。より抽象的な型の配列（ "Pizza displayers"の配列など）を1つ作成し、その配列をFormのプロパティとして保持します。ここではマイナーですが、プログラムのゴミがどれくらいの量であるかを認識することで、コードをより速くすることができます。

浮動小数点に関する注：は、お金のために浮動小数点/倍精度を使用しないでください。代わりに小数点を使用するか、すべてペニーですべてを行います。浮動小数点は正確ではなく、遅かれ早かれ丸めの問題にぶつかります。