Why these jQuery worst practices aren’t.
In this post, Steve Wellens tries to make the case for two common patterns your run across when using jQuery as "worst practices". Practices that are either superfluous or harmful to your code.
1) Wiring up events using unobtrusive JavaScript.
Instead of wiring up your elements events using jQuery, instead you should set the OnClientClick property of the ASP.NET Web Control in question and call whatever JavaScript you want to handle the event.
Instead of this:
-
<script type="text/javascript">
-
-
$(document).ready(function()
-
{
-
$("#Button1").click(function(event)
-
{
-
alert("You Clicked Me!");
-
});
-
});
-
-
</script>
Do This:
-
<asp:Button ID="Button1"
-
runat="server"
-
Text="MyButton"
-
OnClientClick="alert('You Clicked Me!');" />
His reasoning is as follows:
You might want to change event handlers dynamically depending on some condition in the page. If you need to assign a common event handler to several objects on a page, it makes sense to do it in one place. As a matter of fact, that's the beauty of jQuery. But for a single event on a single control….NO.
On can debate whether or not a worst practice can really be applied to a single, specific , one-time event or not. But there are other reasons why you might want to hook up events to a single element using jQuery. one being that you can't hook up multiple event handlers using OnClientClick or HTML's OnClick event, only a single event handler can be assigned. If you are using the observer pattern in your JavaScript, you may have multiple observers that want to subscribe to your buttons click event. But a really strong argument could be made for using the OnClientClick property simply due to ASP.NET's suck-ass way of handling client IDs.
2) Chaining can lead to unreadable code.
In a badly written example, he states that chaining calls to jQuery makes the code unreadable. I'd suggest re-formatting the code in this manner to make it legible and allow for a tighter minimization.
-
$("div.highlight")
-
.css("color", "yellow")
-
.css("background-color", "black")
-
.css("width", "50px");
Of course, you wouldn't write jQuery code like that and call the css method multiple times. Ideally, you would just set a class or at the very least pass in a set of properties in an object.
-
$("div.highlight").css({
-
color:"yellow",
-
background-color:"black",
-
width:"50px"
-
});
3) Comment your code.
-
// get all the Divs with the highlight class and
-
// format them and set their width
-
-
var Divs = $("div.highlight");
-
Divs.css("color", "yellow");
-
Divs.css("background-color", "black");
-
Divs.css("width", "30px");
I like this explanation by Jeff Atwood best. If you need comments to explain WHAT your code is doing, you need to use better names and/or write better code. Comments are best when used sparingly and explain the WHY of the code. Why did you choose to use a particular algorithm
ASP.NET MVC Tip – Return specific views for specific errors.
Earlier I had said to keep your controllers as thin as possible, that doesn't mean that they should necessarily just be two, or one, lines of code.
Take an instance where you are retrieving items from a web service in your controller. Let's say that you get a 404 error and your service will throw an exception telling you that the service can't be found. Take a look at this controller action
-
public ViewResult Item(string itemID)
-
{
-
Item item = MyWebServiceWrapper.GetItem(itemID);
-
return View("Item", item);
-
}
As it stands this action is pretty thin, but if an exception is thrown the user will be presented with either whatever custom error page you have defined in your web.config or the dreaded Yellow Screen of Death.(YSOD). You can wrap the call in a try-catch block and catch the exception. But rather than creating a generic errors page that will display any raw exception message, it's a much better idea to return an error specific view to your user with a friendly error message.
-
public ViewResult Item(string itemID)
-
{
-
try
-
{
-
Item item = MyWebServiceWrapper.GetItem(itemID);
-
return View("Item", item);
-
}
-
catch(ServiceCannotBeReachedException)
-
{
-
Item item = new Item(itemID);
-
return View("CannotRetrieveItem", item);
-
}
-
}
-
-
//The CannotReachService view has a line that looks like this
-
// "The item <%= Model.itemID %> can not be retrieved at this time. Please try again later"
-
// obviously this can be globalized/localized as needed.
This leads to a much "dumber" view and is a much better idea than modifying the "Item" view to display errors if something isn't right with the model or passing in special error parameters to the view. Let the view deal with the data it is meant to display and nothing else.
ASP.NET MVC tip – Don’t use the Content or Scripts directories for view specific files
ASP.NET MVC creates a default file structure for you when you create a new project. It includes a Scripts directory, which contains the MS AJAX and jQuery .js files, and a content directory, which contains a Master page and CSS files. I find this to be extremely cumbersome and it forces me to jump around a lot. Storing shared CSS and JavaScript files in those locations is fine in my opinion. But if you are using View specific CSS or JavaScript files, you should put them alongside your view page in the Views directory. This allows you to quickly find the file you want to work on.
ASP.NET MVC TIP – Keep your controllers and actions thin
When writing you controller actions, keep them short and sweet. Don't add a lot of actions and keep a close eye on the length of each action.
Whenever possible, I try to make my controller actions look something like this.
-
public ActionResult DoSomething()
-
{
-
MyModel model = MyService.GetModel();
-
return View("MyView",model);
-
}
-
//or
-
public ActionResult DoSomethingWithAKey(int myKey)
-
{
-
MyModel model = MyService.GetModel(myKey);
-
return View("MyView",model);
-
}
(1)
There are two reasons to do this:
1) It makes your controllers less complicated and easier to maintain. - You don't want to have to do a lot of debugging in your controllers. Chances are, if you have a lot of complicated logic in your controller actions, you are probably mixing concerns. Make sure that your controllers are only responsible for getting data from your models and figuring out what view to send to the client.
2) It makes testing easier. If all you are doing is getting a model and passing that model to a view, what do you really need to test? Maybe that the correct view is being returned.
(1) Of course I use more descriptive names than "myKey".
Modern JavaScript Development: Reflection in JavaScrpt
Sometimes you want to find out what members an object exposes. There's a pretty simple way to do this. A simple for-in statement.
-
for(var member in obj){ alert(member); };
No really, that's it. No "imports" or "using" statments. No complicated classes to memorize, no unwrapping or casting. Just for-in over an object. One caveat, this will show you ALL of the objects members and it doesn't distinguish between members declared directly on the objects and members inherited through the objects prototype. What if you just want to find the methods on an object? I've created two helper functions that will return just the methods(functions) of any JavaScript object.
-
Object.prototype.getAllMethods = function(){
-
var memberArray = new Array();
-
for (var method in this) {
-
if (typeof this[method] == 'function') {
-
memberArray.push(method);
-
};
-
};
-
return memberArray;
-
};
-
-
Object.prototype.getOwnMethods = function(){
-
var memberArray = new Array();
-
for (var method in this) {
-
if (typeof this[method] == 'function' && this.hasOwnProperty(method)) {
-
memberArray.push(method);
-
};
-
};
-
return memberArray;
-
};
In the getOwnMethods function we use the hasOwnProperty function to discriminate between methods declared in the object and methods inherited through the prototype.
Dealing with comma delimited data in a database column
For various reasons, developers through the years have decided to store lists of data as comma-separated lists. Most programming languages include a split() function that allows you to break apart a list of data using a specified character. T-SQL does not.
I don't remember where I got this split function from. I know I didn't write all of it from scratch. But basically what it will do is take a delimited list of data, split it apart, and return a table where each data element is associated with the identity value you pass into the function. This is very useful if you just want to get the values for a single row.
-
CREATE function Split
-
(
-
@list varchar(8000),
-
@identityVal int,
-
@SplitChar char(1)
-
)
-
RETURNS @RetVal table
-
(
-
id int,
-
Value nvarchar(1000)
-
)
-
AS
-
BEGIN
-
WHILE (Charindex(@SplitChar,@list)>0)
-
BEGIN
-
Insert Into @RetVal (id,Value)
-
Select @identityVal,value = (ltrim(rtrim(Substring(@list,1,charindex(@SplitChar,@list)-1))))
-
SET @list = Substring(@List,Charindex(@SplitChar,@list)+len(@SplitChar),len(@list))
-
END
-
INSERT INTO @RetVal (id,Value)
-
SELECT @identityVal, ltrim(rtrim(@list))
-
RETURN
-
END
So what happens if you are trying to build a view or a report and need to split apart multiple rows. You can iterate over the rows in the table you want to flatten out and call the split function for each row, passing the rows PK into the split function.
-
create function fnFlattenMyTable
-
()
-
RETURNS @RetVal table
-
(
-
PrimaryKeyId int,
-
MyListValue varchar(100)
-
)
-
AS
-
BEGIN
-
Declare @PrimaryKeyId int
-
Declare @MyListColumn varchar(6000)
-
-
-
SET @PrimaryKeyId = (select MIN(PrimaryKeyId ) FROM MyTable)
-
WHILE @PrimaryKeyId is not null --main loop through MyTable
-
BEGIN
-
Set @MyListColumn = (select MyListColumn from MyTable where PrimaryKeyId = @PrimaryKeyId )
-
insert into @RetVal(PrimaryKeyId , MyListValue )
-
select id, value from [Split](@MyListColumn , @PrimaryKeyId , ',')
-
SET @PrimaryKeyId = (select MIN(PrimaryKeyId ) FROM MyTable where PrimaryKeyId > @PrimaryKeyId )
-
END
-
-
RETURN
-
END
This function uses one of my favorite sql tricks. It uses a while loop to avoid having to declare a cursor . The logic is pretty simple, on each loop, select the lowest value of the PK in the table that is higher than the previous PK. That's the roundabout way of saying, "Select the next highest PK from the table".
Right now, this function is hard coded to flatten the table "MyTable", but it could be re-factored into a general use function.


