views:

137

answers:

3

Total beginner question. I am attempting to create a method for a class that calculates the total price for an object based on int Quantity and decimal price. Both of these are private and have Instance Variable Property Assignments. I cannot figure out how to access them and compute on them when I am using two separate parameters in the method. The method in question is the GetInvoiceAmount. Any suggestions would be greatly appreciated

//create invoice class
//intialize instance 

public class Invoice
   {
   public decimal total;  //instance variable to store invoice total

   //public string InitialPartNumber;
   //public string InitialDescription;
   //public int InitialQuantity;
   //public decimal InitialPrice;
   //public decimal InvoiceAmount;
   // auto-imlemented property for class Invoice
   public string PartNumber { get; set; }

   public string Description { get; set; }

   private int quantity; // quantity of items purchased

   private decimal price; // price per item 

   public decimal invoiceAmount;

   public Invoice(string partNumber, string description, int quantity, decimal price)
   {
      PartNumber = partNumber;
      Description = description;
      Quantity = quantity;
      Price = price;


   }//end constructor

   // begin GetInvoiceAmount Method

   public void GetInvoiceAmount()
   {
      invoiceAmount = Price * Quantity;

   }


   //Begin Instance Variable Property Assignment
   public int Quantity
   {
      get
      {
         return quantity;
      } //end get
      set
      {
         if (value >=0 )
         quantity = value;
      } //end set
   }//end property Quantity

   public decimal Price
   {
      get
      {
         return price;
      } //end get
      set
      {
         if ( value >=0 )

         price = value;
      } //end set
   }//end property Price
}//end Invoice class
+2  A: 

Are you looking for something like this?

public Decimal GetInvoiceAmount()
{
    return this.Price * this.Quantity;    
}

Your current GetInvoiceAmount implementation sets the public field invoiceAmount so to use the current method you would need to do something like this:

yourInstance.GetInvoiceAmount();
Decimal amount = yourInstance.invoiceAmount;

which seems counter-intuitive since the method says that it is "getting" something.

For clarity's sake I would recommend that this be a property that is generated each time it is called (that is a property that has no backing field) like this:

public Decimal InvoiceAmount
{
    get { return this.Price * this.Quantity; }
}

Then you can remove the GetInvoiceAmount method and the invoiceAmount field as well as they would no longer be needed.

Andrew Hare
Probably want to remove the parentheses at the end of InvoiceAmount.
Skinniest Man
Yup - thanks :)
Andrew Hare
+1  A: 

What about the following? I made the property setters private to avoid modifying the values after object construction except for the quantity that might change later (it would be even better to use read-only properties, but they first come in C# 4.0 and using read-only backing fields adds quite a bit of noise to the code). You can avoid checking for non-negative quantities by using an unsigned integer. The price gets already checked in the constructor. And finally the total is returned by a computed property.

public class Invoice
{
   // Setters are private to avoid modifying the values.
   public String PartNumber { get; private set; }
   public String Description { get; private set; }
   public Decimal Price { get; private set; }

   // Quantity has public setter and is an unsigned integer.
   public UInt32 Quantity { get; set; }

   // Computed property for the total.
   public Decimal Total
   {
      get { return this.Quantity * this.Price; }
   }

   public Invoice(
       String partNumber, String description, UInt32 quantity, Decimal price)
   {
      // Check for non-negative price.
      if (price < 0.00M)
      {
          throw new ArgumentOutOfRangeException();
      }

      // Maybe check part number and description, too.

      this.PartNumber = partNumber;
      this.Description = description;
      this.Price = price;
      this.Quantity = quantity;
   }
}

USAGE EXAMPLE

Invoice invoice = new Invoice("42-42-42", "Awesome Foo", 24, 42.42);

invoice.Quantity = 42;

Console.WriteLine("Part number : {0}", invoice.PartNumber);
Console.WriteLine("Description : {0}", invoice.Description);
Console.WriteLine("Price       : {0}", invoice.Price);
Console.WriteLine("Quantity    : {0}", invoice.Quantity);
Console.WriteLine("Total       : {0}", invoice.Total);
Daniel Brückner
Thanks for the response!I have to separate the properties into Instance variable property assignments and also use auto-implement property assignments. The Price and Quantity have to use instance variable assignments and are private. I am having difficulty creating the method that will return the invoice amount for each new object that is created by the invoice class
Eric
I like the usage example, this is what mine looks like for initialization:Invoice invoice1 = new Invoice("1234", "Hammer", 2, 14.95M); Invoice invoice2 = new Invoice("5678", "PaintBrush", -5, -9.99M);there is not a value initialized for Total. I have to have a method that creates that for each instance that I can output with the other values.
Eric
You want to initialize the total with an given amount independent of price and quantity?
Daniel Brückner
@Eric, I share Daniel's confusion. You should ensure that your Total property (shouldn't be a method) references the appropriate quantity and price properties.
Nathan Koop
Appreciate you taking a look at this, I did get it to work using the info from astander, however I learned from what you have added, which is always good for a noob.
Eric
A: 

It's not necessary to use the getters within methods of the same class (assuming that the getters do nothing more that just retrieve the private member variable values). The class has total ownership of all the member variables, so it can use them directly.

Unless a getter method does something complicated, there is no reason to protect a class from itself.

Loadmaster