Is this fix to "PostSharp complains about CA1800:DoNotCastUnnecessarily" the best one?

Posted by cad on Stack Overflow See other posts from Stack Overflow or by cad
Published on 2010-04-08T10:10:56Z Indexed on 2010/04/08 10:13 UTC
Read the original article Hit count: 545

Filed under:
|
|
|

This question is about "is" and "as" in casting and about CA1800 PostSharp rule. I want to know if the solution I thought is the best one possible or if it have any problem that I can't see.

I have this code (named OriginaL Code and reduced to the minimum relevant). The function ValidateSubscriptionLicenceProducts try to validate a SubscriptionLicence (that could be of 3 types: Standard,Credit and TimeLimited ) by casting it and checking later some stuff (in //Do Whatever).

PostSharp complains about CA1800:DoNotCastUnnecessarily. The reason is that I am casting two times the same object to the same type. This code in best case will cast 2 times (if it is a StandardLicence) and in worst case 4 times (If it is a TimeLimited Licence). I know is possible to invalidate rule (it was my first approach), as there is no big impact in performance here, but I am trying a best approach.

 //Version Original Code
  //Min 2 casts, max 4 casts
  //PostSharp Complains about CA1800:DoNotCastUnnecessarily
  private void ValidateSubscriptionLicenceProducts(SubscriptionLicence licence)
        {
   if (licence is StandardSubscriptionLicence)
            {               
                // All products must have the same products purchased
                List<StandardSubscriptionLicenceProduct> standardProducts = ((StandardSubscriptionLicence)licence).SubscribedProducts;
                //Do whatever
            }
            else if (licence is CreditSubscriptionLicence)
            {               
                // All products must have a valid Credit entitlement & Credit interval
                List<CreditSubscriptionLicenceProduct> creditProducts = ((CreditSubscriptionLicence)licence).SubscribedProducts;
                //Do whatever
            }
            else if (licence is TimeLimitedSubscriptionLicence)
            {                
                // All products must have a valid Time entitlement
                // All products must have a valid Credit entitlement & Credit interval
                List<TimeLimitedSubscriptionLicenceProduct> creditProducts = ((TimeLimitedSubscriptionLicence)licence).SubscribedProducts;
                //Do whatever 
            }
            else
                throw new InvalidSubscriptionLicenceException("Invalid Licence type");

   //More code...


        }

This is Improved1 version using "as". Do not complain about CA1800 but the problem is that it will cast always 3 times (if in the future we have 30 or 40 types of licences it could perform bad)

    //Version Improve 1
  //Minimum 3 casts, maximum 3 casts
  private void ValidateSubscriptionLicenceProducts(SubscriptionLicence licence)
        {
      StandardSubscriptionLicence standardLicence = Slicence as StandardSubscriptionLicence;
            CreditSubscriptionLicence creditLicence = Clicence as CreditSubscriptionLicence;
            TimeLimitedSubscriptionLicence timeLicence = Tlicence as TimeLimitedSubscriptionLicence;

   if (Slicence == null)
            {               
                // All products must have the same products purchased
                List<StandardSubscriptionLicenceProduct> standardProducts = Slicence.SubscribedProducts;
                //Do whatever
            }
            else if (Clicence == null)
            {               
                // All products must have a valid Credit entitlement & Credit interval
                List<CreditSubscriptionLicenceProduct> creditProducts = Clicence.SubscribedProducts;
                //Do whatever
            }
            else if (Tlicence == null)
            {                
                // All products must have a valid Time entitlement
                // All products must have a valid Credit entitlement & Credit interval
                List<TimeLimitedSubscriptionLicenceProduct> creditProducts = Tlicence.SubscribedProducts;
                //Do whatever 
            }
            else
                throw new InvalidSubscriptionLicenceException("Invalid Licence type");

   //More code...
        }

But later I thought in a best one. This is the final version I am using.

    //Version Improve 2
// Min 1 cast, Max 3 Casts
// Do not complain about CA1800:DoNotCastUnnecessarily
private void ValidateSubscriptionLicenceProducts(SubscriptionLicence licence)
        {
            StandardSubscriptionLicence standardLicence = null;
            CreditSubscriptionLicence creditLicence = null;
            TimeLimitedSubscriptionLicence timeLicence = null;

            if (StandardSubscriptionLicence.TryParse(licence, out standardLicence))
            {
                // All products must have the same products purchased
                List<StandardSubscriptionLicenceProduct> standardProducts = standardLicence.SubscribedProducts;
    //Do whatever
            }
            else if (CreditSubscriptionLicence.TryParse(licence, out creditLicence))
            {
                // All products must have a valid Credit entitlement & Credit interval
                List<CreditSubscriptionLicenceProduct> creditProducts = creditLicence.SubscribedProducts;
                //Do whatever
            }
            else if (TimeLimitedSubscriptionLicence.TryParse(licence, out timeLicence))
            {
                // All products must have a valid Time entitlement
                List<TimeLimitedSubscriptionLicenceProduct> timeProducts = timeLicence.SubscribedProducts;
                //Do whatever
            }
            else
                throw new InvalidSubscriptionLicenceException("Invalid Licence type");

            //More code...

        }


    //Example of TryParse in CreditSubscriptionLicence
  public static bool TryParse(SubscriptionLicence baseLicence, out CreditSubscriptionLicence creditLicence)
        {
            creditLicence = baseLicence as CreditSubscriptionLicence;
            if (creditLicence != null)
                return true;
            else
                return false;
        }

It requires a change in the classes StandardSubscriptionLicence, CreditSubscriptionLicence and TimeLimitedSubscriptionLicence to have a "tryparse" method (copied below in the code). This version I think it will cast as minimum only once and as maximum three. What do you think about improve 2? Is there a best way of doing it?

© Stack Overflow or respective owner

Related posts about c#

Related posts about cast