CSOD ASP.Net webforms



  • The only anonymisation applied is a global replace of a certain word with "Foo".

            protected void ddlSiteGroups_SelectedIndexChanged(object sender, EventArgs e)
            {
                var formView = (FormView)((DropDownList)sender).Parent.Parent.Parent.Parent;
    
                if (formView.CurrentMode == FormViewMode.Edit)
                {
                    ((DropDownList)sender).Parent.FindControl("pnlFoo").Visible = ((DropDownList)sender).SelectedValue == "-1";
                    ((DropDownList)((DropDownList)sender).Parent.FindControl("pnlFoo").FindControl("ddlFoos")).SelectedIndex = 0;
                }
                else if (formView.CurrentMode == FormViewMode.Insert)
                {
                    ((DropDownList)sender).Parent.FindControl("pnlFoo").Visible = ((DropDownList)sender).SelectedValue == "-1";
                    ((DropDownList)((DropDownList)sender).Parent.FindControl("pnlFoo").FindControl("ddlFoos")).SelectedIndex = 0;
                }
            }
    

    This is so packed with WTFs that I'm not even going to bother with the minor ones. Let's look at the biggies:

    • .Parent.Parent.Parent.Parent: because refactoring never happens, so our position relative to the FormView will never change.
    • ((DropDownList)sender) repeated 7 times: because it's impossible to declare a variable within a method.
    • if (condition) { stuff; } else if (other condition) { exactly the same stuff; }: because if (condition || other condition) would make too long a line. (FWIW, in this particular case it's guaranteed that one of the two conditions will succeed, but I can see an argument for defensive coding by not assuming that the third value of the FormViewMode enum will never be used).

  • Trolleybus Mechanic

    Quick primer for why this shit happens. It's almost a textbook example of "canary sniffing" to see if your developer does not understand the framework they're working in.

    (Note: this is a minor simplification, so kepp the dickweed responses to a minimum)

    An asp.net page is broken into two parts-- the "html" file, which has the presentation layer, and the "codebehind" file with all the code, event handlers, etc.  The HTML side is full of ".net" controls-- objects that render out to HTML, but are directly accessible in the codebehind.  So in the example above, "ddlSiteGroup" is presumably a DropDownList, which renders into a <select><option...></option></select> group.  There are public properties on the objects for modifying them, accessing data, altering the way they render, etc.  Like, DropDownList has .SelectedIndex, which tells you the index of the selection Option.

    Easy so far.

    Each .net UI object is also a container object, meaning it can have children inside it.  A Panel (which renders into a <div>) can have other controls in it (which will render into a <div>).  All of these controls (objects) are available at "code time"-- ie: while you're coding. In the codebehind, you can access any control on the page that's set to runat="sever" by it's id.

    HOWEVER, there are many .net controls that have inner controls that are created at runtime.  An asp FormView in this example, is given a dataset, and at runtime will dynamically create and render all the controls inside of it.  Even though they're on the page, they can't be accessed in the codebehind. Because they don't "exist" yet. Even though they're visibly on the HTML page alongside all the other controls.  A junior .net developer will spend hours wondering why Intelilsense won't pick it up, why the compiler complains when they try to reference it by ID.

    The solution is they need to access these controls at runtime by "finding" them-- much like you can insert a node into the DOM via Javascript at runtime, and still find it with document.getElementById().  This is conceptually correct, but implemented so wrong that shit spirals out of control quickly. Here's where it's going wrong.

    When a .net page does a POST, it includes a code which identifies WHICH control caused the POST. It can be any control, even those created dynamically-- like our DropDownList inside a FormView.  .Net recreates the DOM, including all dynamic controls, then finds the ID of the control that did the postback. Each control can specify a function it will use for certain events-- selected index changed, in this case.  Any control that is capable of causing this event can call the function, it's not tied to one specific control. There may be a dozen dropdown lists in the formview, all which call the same function. That is why the first argument, "sender" is an object. It's very generic.

    So here's the coder, writing this function.  They can't refer to the DropDownList that raised the event directly, because it's dynamically created.  Eventually they learn they can cast the object to a dropdownlist.  Now that they know just enough to be dangerous, they get dangerous.

    Since they can't directly access the dropdownlist, obviously they can't access any of the other children of the parent container. And since they can't access those children, OBVIOUSLY they can't access the parent container.  The last part may or may not be true. If the FormView is in itself being dynamically generated, they can't. But they can. They're just doing it in the wrong place, and there's the canary.

    A junior .Net developer will assume the POST event has to be handled in the dropdown list.  It changed. It needs to handle what happens when it changed. Even if it has no visibility to its parent or siblings.  Even if this was the case, a junior developer doesn't know to just iterate up the DOM tree to find the correct parent-- like "elm = sender; while (elm is not a formview) { elm = elm.Parent; }.  That's wrong, but at least "safer".  You still get blind references to the Parent. You still assume the structure of the page above the control handling the event.

    The actual solution is that the dropdown list can raise an event.  (Some controls can even do this directly, rather than catching and rethrowing).  The FormView has several appropriate event handlers. It can catch that event, evaluate the arguments, and handle the logic itself.  Since the event is being handled in the FormView, the "sender" object is the formview. Boom, you have your reference.  The FormView can find its own children without having to worry about their relative positions. Boom, you aren't tightly bound to the UI structure.

    So codeSODs like this are just an artifact of junior coders not understanding the tool they're using. Part of it is that the learning curve for .Net is a bit counter-intuative  (why can't I access it if it's on the screen?!?).  Part of it is coders not understanding fundamental aspects of runtime objects, DOM tress and events.  This is a janky, half-baked solution that's copypasta'd from the Internet, modified without understanding, left alone because it "works", and is a maintenance bomb waiting to happen. It is a screwdriver used as a hammer personified in code.



  • @pjt33 said:

    • .Parent.Parent.Parent.Parent: because refactoring never happens, so our position relative to the FormView will never change.

    Seen that vile construct in WinForms as well, felt the pain :(



  • @Lorne Kates said:

    Quick primer for why this shit happens. It's almost a textbook example of "canary sniffing" to see if your developer does not understand the framework they're working in.

    (Note: this is a minor simplification, so kepp the dickweed responses to a minimum)

    An asp.net page is broken into two parts-- the "html" file, which has the presentation layer, and the "codebehind" file with all the code, event handlers, etc.  The HTML side is full of ".net" controls-- objects that render out to HTML, but are directly accessible in the codebehind.  So in the example above, "ddlSiteGroup" is presumably a DropDownList, which renders into a <select><option...></option></select> group.  There are public properties on the objects for modifying them, accessing data, altering the way they render, etc.  Like, DropDownList has .SelectedIndex, which tells you the index of the selection Option.

    Easy so far.

    Each .net UI object is also a container object, meaning it can have children inside it.  A Panel (which renders into a <div>) can have other controls in it (which will render into a <div>).  All of these controls (objects) are available at "code time"-- ie: while you're coding. In the codebehind, you can access any control on the page that's set to runat="sever" by it's id.

    HOWEVER, there are many .net controls that have inner controls that are created at runtime.  An asp FormView in this example, is given a dataset, and at runtime will dynamically create and render all the controls inside of it.  Even though they're on the page, they can't be accessed in the codebehind. Because they don't "exist" yet. Even though they're visibly on the HTML page alongside all the other controls.  A junior .net developer will spend hours wondering why Intelilsense won't pick it up, why the compiler complains when they try to reference it by ID.

    The solution is they need to access these controls at runtime by "finding" them-- much like you can insert a node into the DOM via Javascript at runtime, and still find it with document.getElementById().  This is conceptually correct, but implemented so wrong that shit spirals out of control quickly. Here's where it's going wrong.

    When a .net page does a POST, it includes a code which identifies WHICH control caused the POST. It can be any control, even those created dynamically-- like our DropDownList inside a FormView.  .Net recreates the DOM, including all dynamic controls, then finds the ID of the control that did the postback. Each control can specify a function it will use for certain events-- selected index changed, in this case.  Any control that is capable of causing this event can call the function, it's not tied to one specific control. There may be a dozen dropdown lists in the formview, all which call the same function. That is why the first argument, "sender" is an object. It's very generic.

    So here's the coder, writing this function.  They can't refer to the DropDownList that raised the event directly, because it's dynamically created.  Eventually they learn they can cast the object to a dropdownlist.  Now that they know just enough to be dangerous, they get dangerous.

    Since they can't directly access the dropdownlist, obviously they can't access any of the other children of the parent container. And since they can't access those children, OBVIOUSLY they can't access the parent container.  The last part may or may not be true. If the FormView is in itself being dynamically generated, they can't. But they can. They're just doing it in the wrong place, and there's the canary.

    A junior .Net developer will assume the POST event has to be handled in the dropdown list.  It changed. It needs to handle what happens when it changed. Even if it has no visibility to its parent or siblings.  Even if this was the case, a junior developer doesn't know to just iterate up the DOM tree to find the correct parent-- like "elm = sender; while (elm is not a formview) { elm = elm.Parent; }.  That's wrong, but at least "safer".  You still get blind references to the Parent. You still assume the structure of the page above the control handling the event.

    The actual solution is that the dropdown list can raise an event.  (Some controls can even do this directly, rather than catching and rethrowing).  The FormView has several appropriate event handlers. It can catch that event, evaluate the arguments, and handle the logic itself.  Since the event is being handled in the FormView, the "sender" object is the formview. Boom, you have your reference.  The FormView can find its own children without having to worry about their relative positions. Boom, you aren't tightly bound to the UI structure.

    So codeSODs like this are just an artifact of junior coders not understanding the tool they're using. Part of it is that the learning curve for .Net is a bit counter-intuative  (why can't I access it if it's on the screen?!?).  Part of it is coders not understanding fundamental aspects of runtime objects, DOM tress and events.  This is a janky, half-baked solution that's copypasta'd from the Internet, modified without understanding, left alone because it "works", and is a maintenance bomb waiting to happen. It is a screwdriver used as a hammer personified in code.



    Wait, doesn't .NET have a "getElementByID()"? Why not just use the id that you know will be rendered eventually to get the object? I don't .NET, so I'm honestly curious.

     


  • Trolleybus Mechanic

    @Snooder said:

    Wait, doesn't .NET have a "getElementByID()"? Why not just use the id that you know will be rendered eventually to get the object? I don't .NET, so I'm honestly curious.
     

    Yes and no. There is .FindControl(string id).  Except it isn't recursive, and cousins can have the same ID.  The recursion problem is a "roll your own once and put it in a utility" solution. 

    In the codebehind, you can only refer to the ID, rather than the "rendered" ID that you see in the html (ie:  ddlSelect inside a panel called pnlWrapper ends up with an ID of ct00_pnlWrapper_ddlSelect). This ID is only known at runtime, so you can't Find it.  Junior coders who don't learn the lesson from above will hardcode this ID into thei javascript (rather than using the server-side control.ClientID).

    So the ID problem is solved by only by using .FindControl on the nearest parent, and you do that by catching the even as early in the DOM as you can, but late enough that the catcher has a child with the unique ID you are looking for.  For example, a GridView (which renders out to a table).  Each row will have a dropdownlist called ddlAction (Open, Delete, Rename).  The dropdown list will raise an event. The GridViewRow (the appropriate <tr>) will catch the even, and can call self.FindControl(ddlAction) and cast it appropriately.

    If the GridView caught it, there are multiple rows with controls with the same ID ddlAction.

    If the Page caught it, there might be multiple GridViews, each with multiple controls of ID ddlAction.


  • ♿ (Parody)

    Eek. I live in a JSF world, and this stuff sounds much worse (though a lot of that is simply unfamiliarity, I'm sure). To do stuff like deciding to make things visible or not (as in the OP's sample), you put stuff in the markup so it calls a method that does whatever calculation or stores the property or whatever. The only time I've ever had to explicitly refer to a control in code was when I had a grid that allowed selection by clicking anywhere in the row, but again, the markup pointed back to something in my code, so there was no ambiguity or wondering how to get a hold of the thing.



  • @Lorne Kates said:

    @Snooder said:

    Wait, doesn't .NET have a "getElementByID()"? Why not just use the id that you know will be rendered eventually to get the object? I don't .NET, so I'm honestly curious.
     

    Yes and no. There is .FindControl(string id).  Except it isn't recursive, and cousins can have the same ID.  The recursion problem is a "roll your own once and put it in a utility" solution. 

    In the codebehind, you can only refer to the ID, rather than the "rendered" ID that you see in the html (ie:  ddlSelect inside a panel called pnlWrapper ends up with an ID of ct00_pnlWrapper_ddlSelect). This ID is only known at runtime, so you can't Find it.  Junior coders who don't learn the lesson from above will hardcode this ID into thei javascript (rather than using the server-side control.ClientID).

    So the ID problem is solved by only by using .FindControl on the nearest parent, and you do that by catching the even as early in the DOM as you can, but late enough that the catcher has a child with the unique ID you are looking for.  For example, a GridView (which renders out to a table).  Each row will have a dropdownlist called ddlAction (Open, Delete, Rename).  The dropdown list will raise an event. The GridViewRow (the appropriate <tr>) will catch the even, and can call self.FindControl(ddlAction) and cast it appropriately.

    If the GridView caught it, there are multiple rows with controls with the same ID ddlAction.

    If the Page caught it, there might be multiple GridViews, each with multiple controls of ID ddlAction.



    Oh, so .NET basically causes the problem by generating multiple elements on the page with the same ID? Why was that considered a good idea?

     



  • @Snooder said:

    @Lorne Kates said:

    @Snooder said:

    Wait, doesn't .NET have a "getElementByID()"? Why not just use the id that you know will be rendered eventually to get the object? I don't .NET, so I'm honestly curious.
     

    Yes and no. There is .FindControl(string id).  Except it isn't recursive, and cousins can have the same ID.  The recursion problem is a "roll your own once and put it in a utility" solution. 

    In the codebehind, you can only refer to the ID, rather than the "rendered" ID that you see in the html (ie:  ddlSelect inside a panel called pnlWrapper ends up with an ID of ct00_pnlWrapper_ddlSelect). This ID is only known at runtime, so you can't Find it.  Junior coders who don't learn the lesson from above will hardcode this ID into thei javascript (rather than using the server-side control.ClientID).

    So the ID problem is solved by only by using .FindControl on the nearest parent, and you do that by catching the even as early in the DOM as you can, but late enough that the catcher has a child with the unique ID you are looking for.  For example, a GridView (which renders out to a table).  Each row will have a dropdownlist called ddlAction (Open, Delete, Rename).  The dropdown list will raise an event. The GridViewRow (the appropriate <tr>) will catch the even, and can call self.FindControl(ddlAction) and cast it appropriately.

    If the GridView caught it, there are multiple rows with controls with the same ID ddlAction.

    If the Page caught it, there might be multiple GridViews, each with multiple controls of ID ddlAction.



    Oh, so .NET basically causes the problem by generating multiple elements on the page with the same ID? Why was that considered a good idea?

    Well in the example that Lorne was giving with GridViews you specify what all goes in a row (the <tr>) in one spot, then you can bind a large collection of data to the GridView and each thing in the collection (say data from a SQL query) gets it's own row.  If you have something like a checkbox or a drop down in each row they each have a different name when rendered on the page, but as that is done when rendering the page you don't want to use those names in the code itself.  Thus you use an internal name (that makes part of the end client name) to reference it.

    Basically it makes simple stuff easier, but you need to be a little more careful when doing something more complex (generally a good trade off as simple stuff is most of what gets done and complicated things generally need to be done by experienced people anyway).



  • And this is why WebForms is awful. All of this idiocy exists because as late as 2008, Microsoft was still actively opposed to the Web. ASP.NET WebForms were built to be as un-weblike as possible. The event driven architecture, with the control-oriented UI design was intended to make web development feel like client side development. The flaw in this logic became painfully apparent when you tried to use their ASP.NET AJAX controls, which were the cruftiest, kludgiest things I have ever had the displeasure of working with. It was a stupid idea back then, and it remains a stupid idea now.

    ASP.NET MVC doesn't have any of these issues, and I'm perpetually stunned by how many .NET developers haven't used it. It's our standard around the office.



  • @Remy Porter said:

    And this is why WebForms is awful. All of this idiocy exists because as late as 2008, Microsoft was still actively opposed to the Web.

    Riiight...

    @Remy Porter said:

    ASP.NET WebForms were built to be as un-weblike as possible.

    You mean it's actually architected by professional designers and not just glommed-together out of 47 different shitty languages?

    I hate to break this to you, buddy, but "as un-weblike as possible" is basically the best selling-point of any software product I can think of.

    @Remy Porter said:

    ASP.NET MVC doesn't have any of these issues, and I'm perpetually stunned by how many .NET developers haven't used it. It's our standard around the office.

    Well I guess you're just that much better than everybody else. They can't match your sheer genius!

    If you honestly believe Microsoft was "actively opposed to the Web" (at least circa 2008?), why the fuck are you using Microsoft web development tools?



  • @Remy Porter said:

    All of this idiocy exists because as late as 2008, Microsoft was still actively opposed to the Web. ASP.NET WebForms were built to be as un-weblike as possible. The event driven architecture, with the control-oriented UI design was intended to make web development feel like client side development.

    Microsoft wasn't actively opposed to the web. Not at all, in fact. They were, however, having to deal with a lot of developers raised on their existing technology stack, some of which were straight-up opposed to programming for the web. So, they created WebForms specifically to ease those developers into web development with something that (at first glance) operates similarly to WinForms.

    The real problem with WebForms is that it is really, really hard to come up with an abstraction that is not leaky. Combined with the amount of work ASP.NET WebForms does under the hood, the relative simple public API and the underlying complexities, it does not take much for the cracks to start showing if developers want to make it do something that lies off the beaten path. If any one word should be picked to describe WebForms' architecture correctly, then it has to be the word "brittle".



  •  This is one of the most thorough explanations I've seen around here. Kudos.

     

    Man, I haven't had a Kudos bar in ages.



  • It could be worse. I maintain ASP.Net Web Forms code where the previous developer uses the auto-generated control names in client side script. If you combine that with JavaScript's default "swallow the exception and stop executing" error handling behavior and you get a nightmare.

    None of the originally posted problem was caused by Web Forms, don't use this as an excuse to attack a platform that works perfectly well for a number of use cases. Personally, I don't use ASP.Net MVC much because any application that has a UI that's too complicated for Web Forms should probably be implemented client-side anyways, so the server framework becomes mostly irrelevant. Also, while HTTP based APIs are fine (REST, WebAPI, etc...), the UI should call the API, it shouldn't be the API. That leaves "ugly URLs" as the main objection to Web Forms, which is easily fixed with URL routing.



  • @Jaime said:

    That leaves "ugly URLs" as the main objection to Web Forms, which is easily fixed with URL routing.

    The URLs are a long way from being my main objection to WebForms. Number 1 as far as I'm concerned is that it takes a nice statically typed system and asks you to do everything by reflection. It's possible to actually take advantage of the type system when using WebForms, but you have to deliberately choose to do things in a non-standard way.



  • @pjt33 said:

    Number 1 as far as I'm concerned is that it takes a nice statically typed system and asks you to do everything by reflection.
    I haven't found this to be the case. I started WebForms development back in 2000 and I have only used FindControl a handful of times in those 14 years.



  • I'm not talking about FindControl: I'm talking about things like BoundColumn / BoundField, ObjectDataSource, etc. In all of the legacy WebForms projects I've worked with it's more laborious than it should be to work out whether a given method is dead code or not, because rather than just checking for references you have to do a full-text search for the name.


Log in to reply