0

I have a class that contains values that are sometimes monetary values (such as 1.25, 2.50, 10.00, etc.) but can also be the string "N/A"; thus, they are declared as string:

public class PriceVarianceDataExtended
{
    public String ShortName { get; set; }
    public String ItemCode { get; set; }
    public String Description { get; set; }

    public string Week1PriceCraftworks { get; set; }
    public string Week2PriceCraftworks { get; set; }
    public string VarianceCraftworks { get; set; }

    public string Week1PriceChophouse { get; set; }
    public string Week2PriceChophouse { get; set; }
    public string VarianceChophouse { get; set; }

    public string Week1PriceGordonBiersch { get; set; }
    public string Week2PriceGordonBiersch { get; set; }
    public string VarianceGordonBiersch { get; set; }

    public string Week1PriceOldChicago { get; set; }
    public string Week2PriceOldChicago { get; set; }
    public string VarianceOldChicago { get; set; }

    public string Week1PriceOldChiFranchise { get; set; }
    public string Week2PriceOldChiFranchise { get; set; }
    public string VarianceOldChiFranchise { get; set; }

    public string Week1PriceRockBottom { get; set; }
    public string Week2PriceRockBottom { get; set; }
    public string VarianceRockBottom { get; set; }
}

However, this code, meant to store the monetary value (as a string) where that makes sense, and otherwise set the val to "N/A":

private string NOT_APPLICABLE = "N/A";
. . .
if (pvde.Week1PriceCraftworks != NOT_APPLICABLE && 
    pvde.Week2PriceCraftworks != NOT_APPLICABLE)
{
    pvde.VarianceCraftworks = 
        Convert.ToDecimal(pvde.Week2PriceCraftworks) - 
        Convert.ToDecimal(pvde.Week1PriceCraftworks).ToString();
}
else
{
    pvde.VarianceCraftworks = NOT_APPLICABLE;
}

...fails to compile, telling me, "Operator '-' cannot be applied to operands of type 'decimal' and 'string'"

So this is my logic:

If the "if" block is entered, that means Week1PriceCraftworks and Week2PriceCraftworks contains numerical values represented as strings, such as "5.00" and "3.14"

I then convert "5.00" to 5 and "3.14" to 3.14 using the Convert.ToDecimal() calls. That should produce a value of 1.86 which is then converted to string ("1.86") so that the correct data type is assigned to pvde.VarianceCraftworks.

But, reasonable and straightforward as that seems to me, I get that err msg. What is wrong with my approach?

BTW, the "week1" and "week2" values are set by calling a method that ends this way:

return price ?? NOT_APPLICABLE;

...so what is contained in Week1PriceCraftworks and Week2PriceCraftworks are either real values (stored as strings) or "N/A".

B. Clay Shannon-B. Crow Raven
  • 8,547
  • 144
  • 472
  • 862

3 Answers3

2

So the answer lies within your

pvde.VarianceCraftworks = 
        Convert.ToDecimal(pvde.Week2PriceCraftworks) - 
        Convert.ToDecimal(pvde.Week1PriceCraftworks).ToString(); // to string is called during the operation rather than afterwards

Secondly I would move some of your code out into extension methods:

pvde.Week1PriceCraftworks != NOT_APPLICABLE

to

public static bool HasValue(this string value)
{
    return value != NOT_APPLICABLE;
}

and

Convert.ToDecimal(pvde.Week2PriceCraftworks)

to

public static decimal ToDecimal(this string value)
{
    return Convert.ToDecimal(value);
}

Then it will read much nicer:

private string NOT_APPLICABLE = "N/A";
. . .
pvde.VarianceCraftworks = NOT_APPLICABLE;
if (pvde.Week1PriceCraftworks.HasValue() && 
    pvde.Week2PriceCraftworks.HasValue())
{
    pvde.VarianceCraftworks = 
        (pvde.Week2PriceCraftworks.ToDecimal() - 
        pvde.Week1PriceCraftworks.ToDecimal()).ToString();
}

This also gives you the ability to change the implementation of HasValue if you choose to go to Nullable<T> etc. etc.

You could even go one step further, with subtracting two known decimal strings:

public static string SubtractDecimals(this string value, string subtractValue)
{
    return (value.ToDecimal() - value.ToDecimal()).ToString();
}

So now:

private string NOT_APPLICABLE = "N/A";
. . .
pvde.VarianceCraftworks = NOT_APPLICABLE;
if (pvde.Week1PriceCraftworks.HasValue() && 
    pvde.Week2PriceCraftworks.HasValue())
{
    pvde.VarianceCraftworks = pvde.Week2PriceCraftworks.SubtractDecimal(pvde.Week1PriceCraftworks);
}

The nice thing about using these extension methods is that you can now read what is going on in plain english, rather than having to read a bunch of implementation details. You can also test these individual extension methods to see exactly what part isn't working how you are expecting if the for some reason the solution isn't working.

Callum Linington
  • 14,213
  • 12
  • 75
  • 154
1
pvde.VarianceCraftworks = 
        (Convert.ToDecimal(pvde.Week2PriceCraftworks) - 
        Convert.ToDecimal(pvde.Week1PriceCraftworks)).ToString();
Hector Davila
  • 180
  • 2
  • 10
1

The issue is the order of precedence based on your parenthesis. It's evaluating to Decimal - String because of the ToDecimal().ToString().

If you change it to the below it should work

(
  Convert.ToDecimal(pvde.Week2PriceCraftworks) - 
  Convert.ToDecimal(pvde.Week1PriceCraftworks)
).ToString();
B. Clay Shannon-B. Crow Raven
  • 8,547
  • 144
  • 472
  • 862
NewDeveloper
  • 301
  • 1
  • 9