+972 52-548-6969
And yet ANOTHER ASP.Net MVC Bug
time to read 2 min | 231 words
This is getting annoying, to tell you the truth. I am trying to develop an application here, not do QA.
Let us take the following class:
public class IdAndName { public long Id { get; set; } public string Name { get; set; } }
And the following code:
var idAndNames = new[] { new IdAndName {Id = 1, Name = "one"}, new IdAndName {Id = 2, Name = "two"} }; var list2 = new SelectList(idAndNames, "Id", "Name", idAndNames[1]); var result2 = Html.DropDownList("test",list2);
What would you expect the value of result2 to be? I would expect it to render a <select> with two items, the second of which should be selected. Here it what it is generating:
<select id="test" name="test"> <option value="1">one</option> <option value="2">two</option> </select>
Notice that there is no selected attribute on the second one.
I will suffice with GRRRRRRRRRR!!!!! at this point.
Comments
This is how the API is intended to be used:
var list2 = new SelectList(idAndNames, "Id", "Name", idAndNames[1].Id);
Ayende, I understanding it is annoying, but you do understand it is beta right? If you find a bug report it so they can work it it and realize that if you are going to develop against a beta product you may have to implement some work arounds.
That being the case, the API is misleading.
The parameter name is selectedValue, not selectedValueId
Even with this explanation, I would argue that this is a bad API.
I don't really care about the ID. I care about the select object.
Joel,
There is beta and there is beta.
This is the sixth release of the ASP.Net MVC.
I expect surface level things to be mostly smoothed out.
Oren, aren't u an MVP?
you had the possibility to connect with the team several months ago, and even now, just send an email to phil or whoever and tell your opinion.
Writing such a post, is an arrogant behavior.
I might have tons of API decision to blog about RhinoMocks, but I'm not writing tons of posts, one per each parameter name I don't like.
I'll have to agree with Oren.
Ayende, I love your blog but your posts have started to really turn arrogant lately.
I hope this doesn't become a consistent behavior.
Why don't you send them a patch for this? ;))
Yes, I think the parameter name is misleading.
However, using the ID is better than using the object. Sometimes you'll have the selected object in hand, and other times you might only have the selected ID in hand. From within in an HTML helper, requiring the ID rather than the object means that's it's easily callable in both cases. If the API required the object, then someone with just the ID in hand would have to search the collection for the matching object, which would dramatically complicate the code calling the HTML helper.
Homeboy, I definitely understand your problems with the MVC but likewise. There have been things I've been frustrated with too but I mean, this sucker is still beta.
That being said, I understand your fears that it may not be ready for prime time once it moves out of beta but still...why all the hate?
HAPPINESS
selectedValue makes perfect sense here.
If it was meant to take idAndNames[1] it would be called selectedItem. This way you could just pass the value without having to find it in the collection.
Marco,
Arrogant? I don't think so.
I am posting, like I always do, about things that I run into when I am working.
Brad,
Sorry I forgot to do it before. Thank you for the quick & accurate comment.
Actually, I disagree with your point on the API name. Look at the rendered markup.
<option value="2" />
Notice the Value is 2 . If you want to select the option with value 2, you pass in a SelectedValue = 2. Not an id. In fact, what does "ID" have to do with anything here except that you just happened to have an ID property on YOUR object.
One thing about MVC is that we're not abstracting away the underlying HTML as much as we do in WebForms. The helpers are just thin wrappers over the HTML markup they generate, so you do have to understand HTML a bit to use these.
I understand that logic. The problem is that it requires understanding the internal semantics of the API to understand how this works.
It would never occur to me that you are actually trying to match things based on the string values that are going to html.
From my point of view, it is an unnatural decision. I would have assumed that you would compare the object to the items in the collection when you generate the list.
I intend to take look at the ASP.Net MVC library, but seeing problems like this is very discouraging. MVC for web aplications was 'invented' 7 or 8 years ago, and I'd expect something more sophisticated than that. Just think about all these layers and classes and C# extensions needed to generate a basic 'select' html. ASP.Net mvc certainly is not a silver bullet.
So if the selected value was coming from a source outside of the collectio you are suggesting that I should need to look up its index in the collection provided first?
The example you are using in your initial post seems unnatural to me, since I can't really see a time where I would hard code the index of the item over the actual value that I wanted.
Sean,
the actual example was something like:
var selectedItem ...;
var allItems = ...;
new SelectList(allItems, "Id", "Name", selectedItem);
@Ayende when we get documentation integrated, that should help a bit. Perhaps a name like "SelectedOptionValue" would be better. Or even "SelectedIdValue" (despite my earlier argument, maybe it's just clearer).
I think Brad clearly outlined why we take in the option value and not the actual object, since it is very common to not have the current value as the full object.
I think, like Phil said, naming it differently would be better. I'd go with SelectedItemValue.
But another idea is making it possible to send either the value or the item - that would make perfect sense, wouldn't it?
Either make an overload for string which would compare against the value and an overload for object which would compare against the item, or make one method that compares against both.
I wouldn't, however call it a bug. Maybe a misfeature?
@Ayende
That does make some sense. Though most of the times I have used it it has been more like:
var selectedValue = VeiwData["categoryId"] ;
var allItems = ...;
new SelectList(allItems, "Id", "Name", selectedValue);
@configurator the problem with that is if we make it a string for selectedItemValue and an object overload for selectedItem, what happens when you do this?
You'd expect that we would use the selectedItemValue overload, but we don't because the Id property is an integer. .NET would call the object overload in this case and treat the integer as the selectedItem not the selectedItemValue.
We've run into a lot of usability problems when we have overload arguments that take in a string or object. People tend to pass in primitive data types expecting the string version to be called all the time.
The real problem here is a method with 4 arguments and very little comprehensibility, even with IntellSense tooltip doc pop-up thingees.
This is where a fluent API or something like that would come in handy because it would be clear what's what:
new SelectListFor(idAndNames).IdIs("Id").NameIs("Name").TheSelectedIdIs(idAndNames[i].Id)
HTML option tag deals with strings, so why is that fourth argument not simply a string?
Even if it's an object, the control could throw an exception on render saying something like "Cannot find item with value 'Rhino.IdAndName' in the select list".
SelectedValue is the value that would be returned by .Selected.Value throughout the .NET API.
SelectedItem is the equivalent name for an actual object currently selected.
[)amien
It's still a whole lot better (and testable) than the .net web forms.
@Chad - what a completely unbiased suggestion! ;)
I had the same problem as you describe. Wondering "why it doesn't work" I had to go to Reflector. If there was a documentation, number of people with this problem would be smaller.
I haven't used MVC so sorry if this is a dumb question but why doesn't it throw an exception if the selectedValue doesn't exist in the dropdown when it renders?
+1 to Neil:
As long as the theme is not to abstract away the underlying HTML, which is nothing but strings, then I think having the 4th parameter as a string would be more self-documenting.
Would this hurt usability? Perhaps, because we now have to call ToString() more often. I can see the challenges in designing the API.
One solution: Have two overloads, one string, one object, and both named "selectedValue". Both overloads does the same thing. The object parameter is there so we don't have to call ToString() everytime.
Another one: Keep the signature as is, but change the behavior to also compare with the instance, not only the value.
Dude, get over it
The API says SelectedValue not SelectedItem.
And yes +1 to Marco
@Phil,
how about comparing to both the item and its value?
I enjoy reading blogs with useful insight. I do not enjoy reading blogs that seem like a public forum for whining. Take up your complaints directly with the team.
I ran into an issue like this that was kind of annoying with the drop down list. If you have an error on your page (IE UpdateModel), it does not set your drop down back to the selected value that came in. I wrote something to get around it, but its pretty freaking ugly.
@Oren
Man all you do is whine anymore. Grow up - or better yet - shut up.
^^the poster above is disgraceful.
Oren, I think SelectedItem would constitute an object, while SelectedValue constitutes a string.
You could create the syntax
var list2 = new SelectList(idAndNames, "Id", "Name", idAndNames[1].Id);
or you could use
var list2 = new SelectList(idAndNames, "Id", "Name", idAndNames[1], "Id");
or
var list2 = new SelectList(idAndNames, "Id", "Name", idAndNames.Select(x=>new {Id=x.Id, Text=x.Name});
But I think I like the original design best
Comment preview