Am I using the set/get function wrong?

So we have two prices here: net (e.g. 45.00) and reduced price (45.00 - 4.50 == 41.50)

public Book {
  ...
  const Decimal PriceThreshold = 30.0m;
  const Decimal ReducePerCent = 10.0m; 

  private Decimal m_NetPrice;

  // Net price
  // Decimal (not Single, Double) usually is a better choice for finance
  public Decimal NetPrice {
    get {
      return m_NetPrice;
    }
    set {
      if (value < 0) 
        throw new ArgumentOutOfRangeException(nameof(value));

      m_NetPrice = value;
    }
  }  

  // Price with possible reduction
  public Decimal Price {
    get {
      return NetPrice > PriceThreshold 
        ? NetPrice - NetPrice / 100.0m * ReducePerCent
        : NetPrice;
    } 
  } 
}

Please, note that we don't have set for Price property; there's ambiguity since one Price, say, 28.80 corresponds to two valid NetPrices (28.80 or 32.00: 32.00 - 3.20 == 28.80)


Why don't you put the logic into the getter. It seems to make more sense since you don't use value in the setter:

public float Price
{
    get
    {
        if (this.bPrice > 30)
        {
            return this.bPrice - (this.bPrice * 0.10f);
        } 
        else
        {
            return this.bPrice;
        }
    }

    private set
    {
        this.bPrice = value
    }
}

EDIT:

a short version of the getter would look like this and (thanks to Patrick Hofman) you can calculate the 90% by multiplication with 0.9:

return this.bPrice > 30? this.bPrice * 0.90f : this.bPrice;


public float Price
{
    get { return this.bPrice > 30? this.bPrice * 0.90f : this.bPrice; }

    private set { this.bPrice = value; }        
}

I made the setter private. Remove it if you want to allow the setting/manipulation of the price also after the creation of your Book object.


You are not using the Setter to set the price. Try the below.

 public Book(string name, string writer, string publisher, float price, string theme)
        {
            Name = name;
            writerName = writer;
            bPublisher = publisher;
            Price = price; // use the setter here, not the member variable 
            bTheme = theme;
        }

If you make the below private, then you will additional protection from people using the wrong variables

private string Name;
private string writerName;
private string bPublisher;
private float bPrice;
private string bTheme;
private float returnValue;

You should also consider making your price a decimal to avoid floating point precision errors.

And as per comment, you need to do a little more work on your property

 public float Price
        {
            get
            {
                return bPrice;
            }

            set
            {
                if (value > 30)
                {
                    bPrice = value - (value * 0.10);
                } 
                else
                {
                    bPrice = value;
                }
            }
        }

Tags:

C#

Class