The Landing: Reflection Diary Unit Four

Work Competed and Learning Outcomes:

Having already been fairly acquainted with JavaScript, what I found most challenging was to find a usable piece of code. After searching far too long, I finally decided to use code that dealt with email validation (the original code as well as the edited code are at the bottom of this document). In my opinion it was poorly written. I edited the code dramatically; it hardly resembles its initial structure. I am quite certain that efficiency was not a priority. It did “work,” however I found that there were some functions with some odd behaviour (or what Kent Beck calls “bad smells”).

My Critique of formValidation.js by Francis Cocharrua:

Initially I noticed that he does not use the “var” keyword. Therefore, every variable that he creates becomes a global variable and is not deleted after the activation object for this function has been deleted. In the words of T.J. Crowder at http://stackoverflow.com/questions/6888570/declaring-variables-without-var-keyword “global variables are a bad idea. They should only be used when absolutely necessary.”
In the function Validate_String, Francis checked for a null or empty string after variable (global variable) assignments. I found this peculiar. It is unnecessary to assign variables for a conditional statement that if true will effectively terminate the function.
In the variable valid_chars I noticed that both uppercase and lowercase letters were included instead of a call to String.toLowerCase (or toUpperCase). This affects program efficiency in two ways. First it increases the necessary length of the string. Second it significantly prolongs the length of the comparison between the email address and valid characters. The comparison must be run 26 extra times for every character in the email address. So for an email with 15 characters that is 390 unnecessary comparisons.
The for loops use a global variable index.
The return value for Validate_String is either true, false or a string. In my opinion I think this is poor practice simply because it’s confusing. Regardless Validate_Email_Address has no way of dealing with a string as a return value. Perhaps creating another function invalidCharacters could return a string value, but I don’t think it one function should do both. That is my opinion though.
The string function substr was used to return a character at a given position instead of the charAt function.
Loose equality conditional statements are used instead of strict for strict purposes.

Rationale for What Has Been Done:

Honestly, I decided it was necessary to complete this unit eventually. Instead of trying to find the perfect code to apply, I would edit said code to my hearts content. And that I did. I am quite pleased with the result as well. I am in no way disappointed with my experience. I found the ordeal to be invaluable; the lessons were worthwhile.

I am curious to see my feedback about commenting on this piece of code. There are actually very few comments in total, however I believe that the result is actually very readable to a somewhat experienced programmer. For example:

if(!vaildCharacters(emailAddress) ||

!vaildDomain(emailAddress))

return false;

This statement is fundamental to the functionality of the program and there are no comments. However, the code itself explains the functionality. It is read:

If not valid characters email address, or not valid domain email address, return false.

Which would essentially be the result of writing pseudocode. I was thinking about creating a function called “not” which would simply be

function not(arg){return !arg;}

So that this type of code would be even more clear:

If(not(validCharacters(emailAddress)))…

I’ve adopted this style from Martin Fowler and Kent Beck in the book Refactoring: Improving the Design of Existing Code, theysay:

“The real key to making it easy to understand small methods is good naming. If you have a good name for a method you don't need to look at the body. The net effect is that you should be much more aggressive about decomposing methods. A heuristic we follow is that whenever we feel the need to comment something, we write a method instead. Such a method contains the code that was commented but is named after the intention of the code rather than how it does it.”

The Good and The Bad:

I really cannot say anything negative about this experience. Yes, the code was below par (in my opinion), but no one has the same coding style. It can often be difficult to judge different styles as better or worse. This assignment really allowed me to look at the fundamentals of programming; one of my heroes Jim Rohn says:

“Little mistakes repeated over time lead to disaster. Should change, could change, don’t change. I’m telling you it will lead to disaster.”

And it applies to a whole lot more than just programming. Make no mistake, little inefficiencies scattered throughout a program may seem insignificant now, but who knows one hundred lines from now, or ten thousand lines from now. I implore you to think of the final product, what will ten bytes here, three bytes there lead to? That lag could lead to a visitor leaving before they have a chance to see the material.

What I Would Have Done Differently:

In retrospect, I spent a lot of time looking for the perfect piece of code. I realize now, that I must write such a piece. Who in the World besides me is going to write the code for my site (well unless I pay or inspire them to). There’s a good chance that I could have saved about an hour if I had a slightly different perspective on the subject.

JavaScript Files and Description:

The code is very straight forward. It checks whether the characters of an email address (given by the input field "mail") are valid, and that the at sign and period are used conventionally. It verifies that:

Contains an at sign
Has characters before the at sign
Contains a period
Period is after, but not immediately after the at sign

An event listener is then created for when the input field "mail" loses focus. If these conditions are not met, the elements border color shifts to red. If the conditions are met then the border color changes (or remains) blue.

Original