Skip to content

Bad practice: 2 in 1

8 July 2011

Recently reading a book I’ve run across a code snippet that is absolute example of bad practice for me. It is quoted at the end of this post.

Why is bad? What’s the problem?

Well, first of all using .NET code (C# in the case) to write a simple javascript is absolute nonsense. If we take this idea and develop it further we could use another language, let’s say C++ to write a C# code to write javascript code. And we could continue on and on until we use up all existing languages.

I agree this will make our code fancy, but what about the other stuff: OOP, re-usability, easy supported… you name it.

On the other hand so written javascript code is hard to read, harder to support and even harder to fix bugs caused by typos for instance. Javascripts should be added as a .js file (like .cs files)  and should be kept clean and separate from the other code.

Bad practice exists also in the javascript itself:

  • passlen variable is hard coded inside the function as it should be a pass-in parameter.
  • a bunch of else if statements are supplied causing check and check and check … Slow! Bad!
    Each else if should be replaced with case in one switch statement, which is easier to read and faster.

to name few of them… You’ll find perhaps others.

At the end I hope this writing to reduce developers’ headaches caused by such pitfalls.

The code
public partial class PasswordCheckTest : System.Web.UI.Page 
{ 
    protected void CheckBoxPassStrengthOn_CheckedChanged(object sender, EventArgs e) 
    { 
        if (CheckBoxPassStrengthOn.Checked)  
        { 
            System.Text.StringBuilder passFunc = new System.Text.StringBuilder(); 
            passFunc.Append("function CheckPassword() {"); 
            passFunc.Append( @"var passLen = document.forms[0].MainContent_ 
  TextBoxPassword.value.length;"); 
            passFunc.Append(@" if (passLen < 4) {"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"innerText = ""weak"";"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"style.color = ""red"";}"); 
            passFunc.Append(@" else if (passLen < 6) {"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"innerText = ""medium"";"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"style.color = ""blue"";}"); 
            passFunc.Append(@" else if (passLen > 9) {"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"innerText = ""strong"";"); 
            passFunc.Append(@" document.getElementById(""passwordStrength"")."); 
            passFunc.Append(@"style.color = ""green"";}}"); 
            //register the script 
            Page.ClientScript.RegisterClientScriptBlock(this.GetType(), 
              "CheckPasswordScript", passFunc.ToString(), true); 
            //add an event to the text box to call the script 
            TextBoxPassword.Attributes.Add("onkeyup", "CheckPassword()"); 
        } 
            else 
        { 
            //remove the event from the text box 
            TextBoxPassword.Attributes.Remove("onkeyup"); 
        } 
    } 
}
Advertisements

From → Tips

Leave a Comment

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: