views:

336

answers:

3

What are some good ways to deal with this messy and unnecessary and not-really-dynamic generation of JavaScript:

var <%# JavascriptId %> = new BusinessChart(
    '<%# JavascriptId %>',<%# CurrentUserId %>,'<%# ChartId %>'
    ,'<%# Helper.GetBaseUrl() %>','<%# ChartPath %>'
    ,'<%# Helper.ResolveUrl("~", true) %>'
);

<%# JavascriptId %>.Init();

I found this other question, but the answers don't seem to address the source of the smelliness.

I see a few specific issues:

  • JavascriptId is a variable name. Why should I ever, EVER define a client-side variable name on the server side?
  • The CurrentUserId doesn't ever change for the user... it's their user ID. Same for GetBaseUrl() and ResolveUrl("~")... Why should I pass constants all over the place?
  • I have to open the aspx.cs codebehind file to debug stuff, and can't use Intellisense.

I have developed a couple ideas to deal with the above issues (declaring a global "Application" object, jQuery + declaring classes on DOM elements), but I'd like to hear more thoughts on this.

Thanks for any suggestions you might have.

+1  A: 

You're asking about code smells, so the I guess the vagueness of the code situation is appropriate. What's the deal with BusinessChart, for example. There's a huge amount we don't know here. But here's what I smell: Only the first issue you mention smells really bad to me. Very strange to have to specify that on the server. I suppose there could be a reason for that, but I have a hard time picturing it. As for the variable CurrentUserID, there could easily be good reasons for that. Maybe BusinessChart filters on different data depending on the user's role, for example.

As for GetBaseUrl and ResolvUrl, that could be legitimate also. BusinessChart might need a fully qualified URL, and GetBaseUrl/ResolveUrl are a central place to provide that so you only need to make the configuration change in one place. And why not use a web.config reference for that? Errrr, maybe there's multiple web apps or deployment that use these paths, and the Helper class gets these URL's from a common db, providing one common configuration place for multiple apps or deployments.

As for using the codebehind . . . sometimes that's necessary. Though it's true that often dynamic code like this is unnecessary, sometimes having the complication there achieves the greatest overall simplicity.

I try to give the benefit of the doubt to existing code, as you can see. However, I wouldn't be surprised to find that, as you suspect, all the dynamic code in your example is indeed absolutely useless. I would say that your sense of smell seems pretty good! And that the first issue you state smells the worst.

Patrick Karcher
+1  A: 

I don't know if it solves your exact problem but this article by Rick Strahl may be of interest.

Robert W
Thank you - this is a good example what I was looking for. The comments section of that article links to another blog that syncs JS and C# variables... neat!
Jeff Meatball Yang
+1  A: 

Perhaps you should focus more on data-driven JavaScript. Store your data as JSON, and leave your code to do "codey" things:

// This is the dynamic data part
var charts = [{
  Id: '<%# JavascriptId %>',
  UserId: <%# CurrentUserId %>,
  ChartId: '<%# ChartId %>',
  BaseUrl: '<%# Helper.GetBaseUrl() %>',
  ChartPath: '<%# ChartPath %>',
  HomePath: '<%# Helper.ResolveUrl("~", true) %>'
}];

// This is just code that can be stored away 
// in the application's static javascript
function initChart(data) {
  var chart = new BusinessChart(
      data.Id, data.UserId, data.ChartId, 
      data.BaseUrl, data.ChartPath, data.HomePath);
  char.Init();
  return chart;
}

Even better, instead of putting this in the HTML file, write a REST request handler that returns this JSON.

Now you're very close to an AJAX application.

Frank Krueger
Your JSON request handler is how I proceeded with this, and yes, the direction we are heading certainly breaks down into static JS code that consumes data-driven JS objects. Thanks.
Jeff Meatball Yang