current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

There are 28 check boxes that are given a common name and have different values:

<input type="checkbox" name="Schedules" id="checkbox1" value="1">

In controller in post action:

public ActionResult Create(SubscriptionPlanCreateViewModel subscriptionPlanCreateViewModel, FormCollection Form)
{
    string[] AllStrings = Form["Schedules"].Split(',');
    foreach (string item in AllStrings)
    {
        int value = int.Parse(item);
        if (value == 1)
        {
            subscriptionPlanCreateViewModel.Timeschedule = "OneTime";
        }
    }
}

By using this code I need to use 28 if conditions. How can I reduce the code size?

share|improve this question

migrated from stackoverflow.com Nov 15 '11 at 13:53

This question came from our site for professional and enthusiast programmers.
    
Are all the if conditions trying to set the Timeschedule property? Or are they all setting different properties on your view model? Also, do you have any control over how the view is generated, or are you restricted to just changing the controller code? –  StriplingWarrior Nov 15 '11 at 5:46
    
@StriplingWarrior No value==1 for time schedule for serp and passes value "onetime" to Timeschedule then value==2 passes value="Weekly" to timeschedule etc –  Sreenath Plakkat Nov 15 '11 at 5:50
    
So what if both checkbox 1 and 2 are selected? –  StriplingWarrior Nov 15 '11 at 5:54
    
if both the value 1 & 2 are selected the timeschedule is appended with value of 2 if (value == 1) { subscriptionPlanCreateViewModel.SerpRankingSchedule = "OneTime"; } if (value == 2) { subscriptionPlanCreateViewModel.SerpRankingSchedule = (subscriptionPlanCreateViewModel.SerpRankingSchedule == null || subscriptionPlanCreateViewModel.SerpRankingSchedule == "") ? "Weekly" : subscriptionPlanCreateViewModel.SerpRankingSchedule + " | Weekly"; } –  Sreenath Plakkat Nov 15 '11 at 5:57
    
the first 3 values are for SerpRankingSchedule then the next 3 values are for Reports schedule etc –  Sreenath Plakkat Nov 15 '11 at 5:59

2 Answers 2

up vote 2 down vote accepted

You can use the checkboxes like this

<input type="checkbox" name="Timeschedule" id="checkbox1" value="OneTime">
<input type="checkbox" name="Timeschedule" id="checkbox2" value="Weekly">

and in the controller action the values from multiple checkboxes can be retrieved by using this method

string[] AllStrings = Form["Timeschedule"].Split(',');

or 

string[] AllStrings = Request["Timeschedule[]"].Split(',');
share|improve this answer
up vote 3 down vote

It seems to me that what you really need to do is change the view code. For example, if you do this:

<input type="checkbox" name="Timeschedule" id="checkbox1" value="OneTime">
<input type="checkbox" name="Timeschedule" id="checkbox2" value="Weekly">

That will automatically bind to the Timeschedule property on your SubscriptionPlanCreateViewModel. However, if the user selects multiple checkboxes, the values will probably come across as comma-separated. You can either make Timeschedule an array (which would probably better represent what you're doing from a data perspective) or you can simply replace the commas with " | " to produce the string you say you want in your comments.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.