Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

A colleague and I were developing an ASP.NET MVC project and during this we created a new action method to handle one of our views. I initially started created the action like so:

public ActionResult Index(int id, int numberOfAssessments, int age)
{
    // do stuff
}

However when I started writing we had a little discussion:

Him:"Why am I making these ints, they should all be strings"

Me: "Well they are ints so it makes more sense to make them strongly typed and so stop potential errors later in the code block".

Him: "Back what if we wanted to change the id to a alpha-numeric representation. We know have to change this signature everywhere where if it was a string it would already handle this".

Me: "It's not a string now and we don't have any plans for making it a string at this moment though".

We didn't really decide explicitly what to do but it did get me thinking.

Question:

Should parameters to Action methods be all strings if that is going to make it more flexible in the future, or should they be specific to their type so that we can leverage MVC binding and error handling? What is the best or accepted practice in these situations.

Are there too many considerations to take in for any definitive answer?

share|improve this question
    
Hopefully you thought about your design well enough so that such radical changes to the design are not needed. And changing the type of a fundamental identifier is a radical design change. –  Robert Harvey Sep 4 '13 at 22:47
    
@RobertHarvey so if both satisfies the need i.e. string and int I guess it's in general better to go with the one that provides the more explicit definition of the type i.e. if id is an int, then accept an int? –  dreza Sep 4 '13 at 23:11
    
Tossing strings around is just going to require you to perform numeric conversions all the time. Might as well let the framework take care of that housekeeping for you. –  Robert Harvey Sep 4 '13 at 23:12

3 Answers 3

You're actually asking two different questions here.

  1. Should MVC parameters all be strings?

    Of course they shouldn't. The fact that you may want to change the type later is an absurd argument; somewhere, somehow, some component is going to be depending on that ID being a specific type, and it's not going to be any easier or less risky in the future to change a bunch of casts and type checks than it is going to be to change the type of a parameter.

    If the designers of ASP.NET MVC didn't want you to use strong-typed parameters (or the more sophisticated equivalent, model binding with strong-typed properties) then they wouldn't have written a ton of code to support exactly that scenario. Strong typing is one of the main reasons to use a framework like MVC, instead of legacy frameworks where you had to screw with Request.QueryString and so on.

    If you miss the good old days of Request.Form and Request.QueryString and don't think you'll ever need to overload any of your action methods, then by all means turn all your parameters into strings for nostalgia. If you still have some measure of sanity left, you'll leverage the extensive validation that's built into MVC for your convenience and use the correct types.

  2. Should IDs be strings?

    This is a much more difficult question to answer because very often it depends on the ID.

    A lot of the time you may have different developers and/or different teams working on the same product or feature. Often there will be domain model code and/or APIs written before anyone has any clue about how or where it's going to be stored. Other times you'll be working in a DDD/BDD style and want to express the fact that it really doesn't matter what type it is because there's no validation associated with it.

    You see, web APIs lend themselves very well to the string-as-ID model, because no matter what the underlying type turns out to be, the error handling is trivially simple: Try to convert to the expected type, and if conversion fails, then return a 404. An invalid ID is functionally the same as a missing resource.

    The above doesn't work if you allow the client to specify its own IDs for new resources, but then again, if you're going to allow the client to specify its own IDs, you really should make the ID a string instead of imposing arbitrary and probably undocumented restrictions on the client. Non-string IDs are usually non-strings specifically because they need to be auto-generated, e.g. an integer sequence, a GUID, a BSON Object ID, etc. If your ID isn't auto-generated, a string is typically the best choice for your underlying data model because it allows for the most human-readable IDs.

    I prefer to use string IDs in my API and/or view models because it saves me having to worry about what the database guys are doing and/or potentially have to face a type change. But that is merely personal preference from my own experience and certainly not a hard and fast rule about MVC; there are certain frameworks/products that have real trouble with string IDs (RavenDB, I'm lookin' at you) so it's not a decision you should make lightly, not unless you can understand and predict the consequences.

So there is some merit to the argument about making the ID a string - but not to making all of the parameters strings for no real reason. That's just weird and I'd take a hot steaming dump on it if I ever saw it in a code review.

share|improve this answer
    
This is what I wanted to say, but couldn't think of a nice way of wording it. I'm not sure if "hot steaming dump" is nice, but it fits. :) –  Eric King Sep 5 '13 at 15:29
    
"If you still have some measure of sanity left..." Made me smile. Thanks for this writing this up. It's gonna save me a lot of arguing :) –  Marjan Venema Sep 5 '13 at 16:29

Should parameters to Action methods be all strings if that is going to make it more flexible in the future, or should they be specific to their type so that we can leverage MVC binding and error handling?

In short: No, each parameter should represent the type that your method expect. Otherwise, you would need check each parameter and cast them to specific types within the method.

As an example, take the DateTime type. If you pass it as a string then you would need to have casting to get the actual DateTime value, while MVC model binding provides that out of the box.

Currently, our team is working on a large asp.net mvc application, and we follow several good practice in the project. One of them is to allow nullable types for parameters that may have null values, when the requested or posted. This approach helps a lot with optional search parameters that end user may/may not provide.

share|improve this answer

I'd suggest that you only write the code to solve your current problem and your current problem only.

If taking advantage of the MVC binding, client-side validation, etc. makes solving your current problem easier, then by all means go with what addresses the current problem that you're trying to solve.

And just in case I wasn't clear before, only focus on your current problem.

share|improve this answer
    
So both ways to be honest solves the problem. Which way would be the recommended? –  dreza Sep 4 '13 at 22:39
1  
This is a bit of a dodge. No self-respecting programmer would butcher an interface, or patch around a perfectly good class, just because it solves "your current problem." –  Robert Harvey Sep 4 '13 at 22:45
    
@Robert... IMO, the idea of using all strings and dealing with the overhead that causes and the lack of built-in functionality just to be prepared "in-case we change it later" is closer to butchering the interface than simply doing what's required to address the current problem. –  TMc Sep 5 '13 at 3:31
    
@draza... both ways don't actually solve the problem, as I see it. One way reduces your ability to take advantage of built-in functionality, thereby creating additional problems to be solved. –  TMc Sep 5 '13 at 3:32

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.