6

I have a model that has an integer property. When the model is submitted with 23443, the model binder works great and the value is available in the action. But if the model is submitted with a thousands separator like 23,443, the value isn't parsed and the property is zero. But I found that a property with the type of decimal could have the thousands separator and it would parse and be populated correctly.

I found out that by default Int32.Parse() doesn't parse thousands separator but Decimal.Parse() does allow thousands separator. I don't want to have to write a check like:

public ActionResult Save(Car model, FormCollection form) {
    Int32 milage;
    if(model.MyProperty == 0 && Int32.TryParse(form["MyProperty"], NumberStyles.AllowThousands, CultureInfo.InvariantCulture, out milage) {
        model.MyProperty = milage;
    } else
        ModelState.AddModelError("Invalid", "Property looks invalid");

    [...]
}

every time I deal with these fields. It looks ugly, and moves all the validation out of the model attributes. Changing the type of the property to decimal just to make the model binding work doesn't seem like a smart idea. When I look into the model binder, it looks like it's using TypeConverter to complete the conversions from string to the type. And it looks like Int32Converter uses Int32.Parse() with NumberStyles.Integer.

Is there a way to change the behavior of Int32Converter to allow the thousands separator to be parsed by default? Maybe override the default NumberStyles on Int32.Parse() across the entire app? Or is adding my own model binder which parses the integers with NumberStyles.AllowThousands the only/correct course of action?

Steven V
  • 16,357
  • 3
  • 63
  • 76
  • I'm not sure what the answer to your question is, but it's likely you'll run into similar formatting problems with other data types (perhaps DateTime's). My recommendation is to use a string property in these situations and parse it yourself, rather than trying to coerce the model binder into behaving differently. – Ryan Jan 17 '14 at 19:47
  • @Ryan Hmm... are you saying that should update the model property to be a string, instead of an number type? – Steven V Jan 17 '14 at 19:50
  • Meaning, you have a `public string MyNumber {get;set;}` which catches what the user actually submitted, and an `internal int ParsedMyNumber { ... }` - and inside that one, you parse the string value in the getter (stripping the thousands separator). – Ryan Jan 17 '14 at 19:52
  • You could also create a CarViewModel so that your Car model stays nice and clean. If you always put a view model between your view and your model, you always have a handy place to put things like this. – jimmyfever Jan 22 '14 at 16:13
  • @jimmyfever Indeed. And the application is designed that way, just sample code I wrote for the question. But to populate the view model, I'd run into the same set of issues. – Steven V Jan 22 '14 at 16:25
  • 1
    How about a custom model binder attribute? You can then decorate any int on your model that need to be parsed with AllowThousands easily and cleanly - I just think that a model binder for the entire type might be a little bit over kill - especially if you have other types that need to have the int parsed with AllowThousands. – David McLean Jan 24 '14 at 10:00
  • @DavidMcLean You should post that as an answer instead of a comment. I completely forgot about the binder attributes. My only complaint would be a developer would need to remember that exists, instead of it happening automatically. But I will definitely investigate! – Steven V Jan 24 '14 at 15:13
  • Could you just remove all the commas before parsing the string as an int? – David R Tribble Jan 27 '14 at 20:51
  • @DavidRTribble Well, I was trying to use the magic of the model binder so I wouldn't have to do parsing in my actions. But seems like the way to go is a custom model binder that can do that parsing. – Steven V Jan 28 '14 at 03:01

1 Answers1

0

I think, you can add custom binder for int type.

Demo: http://dotnetfiddle.net/VSMQzw

Useful links:

updated

Based on Haacked article:

using System;
using System.Globalization;
using System.Web.Mvc;

public class IntModelBinder : IModelBinder
{
    #region Implementation of IModelBinder

    public object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext) 
    {
        ValueProviderResult valueResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName);
        ModelState modelState = new ModelState { Value = valueResult };
        bindingContext.ModelState[bindingContext.ModelName] = modelState;

        object actualValue = null;
        try 
        {
            actualValue = Int32.Parse(valueResult.AttemptedValue, NumberStyles.Number, CultureInfo.InvariantCulture);
        }
        catch (FormatException e) 
        {
            modelState.Errors.Add(e);
        }

        bindingContext.ModelState.Add(bindingContext.ModelName, modelState);
        return actualValue;
    }

    #endregion
}

And in the Application_Start event (probably in the Global.asax), add:

ModelBinders.Binders.Add(typeof(int), new IntModelBinder());
Community
  • 1
  • 1
hardsky
  • 514
  • 5
  • 15
  • I did not test it in any real asp.mvc project. – hardsky Jan 26 '14 at 14:04
  • A custom model binder had crossed my mind. I wasn't sure if there was a better way than doing that though. Also, can you edit your post to include the code? In the event dotnetfiddle goes down, the code is lost forever. [More information](http://meta.stackexchange.com/q/8259) – Steven V Jan 27 '14 at 14:14