Why is this considered bad practice? or that? (ASP.Net)
Would this code be considered bad practice:
<div id="sidebar"> <% =DisplayMeetings(12) %> </div> This is a code snippet from default.aspx of a small web application I was working on. It works fine, it works very fast, but, as always, I know that just because it works does not mean that everything is in order.
Basically, the DisplayMeetings routine outputs a bunch of formatted HTML (actually an unordered list), without formatting, only the required html, and then my CSS does all the necessary formatting.
The data for generating the list comes from the SQL server database (the parameter controls how many rows will be returned), and I use stored procedures and datareader for quick access. This makes my frontend unusually simple and clean, imho, and allows me to do all the work in VB or C # in a separate module.
I could, of course, use a database-bound repeater (and possibly 6 or more other methods) to do the same thing, but are they better? Besides the loss of features in the development time of VS2010?
The only “wrong” thing with your approach is that it mixes code and display, which you usually avoid when possible. If you need to have a part of the HTML processed by the procedures (because it is simply difficult with controls or something else), create the control responsible for creating this HTML and paste it into the large page / control that contains it.
is the "wrong" part that the routine returns HTML, or is it that I have a piece of code made from HTML markup?
Both, in a way. And also.
There is nothing direct "wrong" using <%= foo %> ; if they were, it would not be part of the structure. What is "wrong" in it is that it establishes a two-way dependency that you do not necessarily want. Your HTML markup is dependent and should know about the code. It should call a method, which, in turn, is intended for markup and nothing else. If you want to make changes to your output, you may need to change the method, markup, or both.
What I'm trying to say is that what you do makes your code less maintainable, less flexible to change the next time you need to open the hood.
If this is the only way to solve the problem, then this is the only way, and there is nothing wrong with that. But this is what should be avoided, in general, if possible.
How would I do that? Honestly, it depends on the situation. If I could use a standard component with data binding, I would do it. It is always preferable. If HTML was too complicated to use the component, I would use the Literal control as a placeholder for the markup and create the markup in a separate component - probably User Control.
The fact is that the markup does not know anything about the method used to generate another markup, it just says “something goes here” and relies on its code to cope with deciding which markup to put there.
This is not what I would do. I am not a fan of using <%= %> in Webforms. Unfortunately, this is also a common practice.
Instead, I would try to create a custom control to present the markup I wanted and put it on the form. This makes it possible to make the element more reusable, testable and gives you more control over where the code is called in the life cycle of the page.
A data follower can make it a little easier to change your html when you need to change the layout. It is also nice if you have individuals who work on html and people who work with server-side code.
My usual rule is to use a relay for any even slightly complex html. And I do not call methods from my aspx / ascx file - I only insert the values of the protected string variable that I filled in the code. However, these are personal preferences, and I see nothing wrong with what you do.
Your code (without looking at it) will most likely be faster than a relay, as there is no data binding, but it probably will not be a huge thing if the page is very, very popular.