How to not use an if else statement when I get the type of a model...

yguyon28
yguyon28 used Ask the Experts™
on
I have the following if statement that I'm returning a model if it match my current model. I'm trying to re write the example bellow without an if else statement. Any suggestions would be great...

        public ActionResult ReportWriter(ReportRequestViewModel viewModel)
        {
            bool hasAccess = true;
            if(!hasAccess)
            {
                return RedirectToAction("Index", "Reports", new { area = "Reports" });
            }
            ViewBag.IsAngularJS = false;
            var model = GetViewModel(viewModel.ReportType.ToString(), viewModel.ConfigurationId);
            var type = model.Data.GetType().Name;
            ReportViewModel vm = null;
            #region ViewModel Casting
            if (type.Equals("SalesReportViewModel"))
            {
                vm = model.Data as SalesReportViewModel;
            }
            else if (type.Equals("CustomerReportViewModel"))
            {
                vm = model.Data as CustomerReportViewModel;
            }

Open in new window

Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®

Commented:
You could use a ternary; e.g. -
public ActionResult ReportWriter(ReportRequestViewModel viewModel)
{
    bool hasAccess = true;
    if (!hasAccess)
    {
        return RedirectToAction("Index", "Reports", new { area = "Reports" });
    }
    ViewBag.IsAngularJS = false;
    var model = GetViewModel(viewModel.ReportType.ToString(), viewModel.ConfigurationId);
    ReportViewModel vm = model.Data is SalesReportViewModel ? model.Data as SalesReportViewModel : model.Data as CustomerReportViewModel;
}

Open in new window

-saige-

Commented:
Just cast it as a ReportViewModel, since that's what vm is (it won't lose the original type - it's simply using the parent class as a container). Also, use the "is" operator instead of running GetType().Name and comparing it to a string (which forces .NET to create an instance of that string just for the sake of comparison, which is inefficient).

var model = GetViewModel(viewModel.ReportType.ToString(), viewModel.ConfigurationId);
ReportViewModel vm = null;
if(model.Data is ReportViewModel)
{
  // This works since SalesReportViewModel and CustomerReportViewModel are both sub-classes of ReportViewModel
  vm = (ReportViewModel)model.Data;
}

Open in new window


The object doesn't actually change, so you can still check the exact type later if you need to. For example, later on if you want to run something specific if it's a SalesReportViewModel, you can still do:

if(vm is SalesReportViewModel)
{
   var salesReportViewModel = (SalesReportViewModel)vm;
   ...code specific to SalesReportViewModel...
}

Open in new window

Miguel OzSenior Software Engineer
Top Expert 2009

Commented:
I agree with gr8gonzo that you only need to cast it as a model, but my suggestion is to check how the model is build.
Some questions:
Q1. What is the model.Data type?
Q2. How vm variable is used in the code?
Q3. Are all targeted models (SalesReportViewModel, CustomerReportViewModel) derived from ReportViewModel?
If not why not.

If the base class of of your models is ReportViewModel and your vm variable code is use as ReportViewModel , then there is no need for casting at all just make sure model.Data  type is ReportViewModel and your code can be simplified.


Note: If more help needed please answer questions above and post code for GetViewModel

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial