The regular expression cookie hammer

I am guilty of it, you are guilty of it; writing a piece of code that makes someone want to cry. This time I am thankful it is not my code.

The problem we are trying to solve this time is as follows:

There is a <div> with content that scrolls vertically.  When a postback happens, the content in the <div> will be refreshed and the scroll position is reset to the top (scrollTop becomes 0).  The scroll position should not jump around, but rather should remain in the same position.

A solution submitted for your disapproval:

function LoadScrollPosition(scrollableDiv, isPostBack) {
 if (false == isPostBack) {
  ResetScrollPosition(scrollableDiv);
  SaveScrollPosition(scrollableDiv);
 }
 else {
  RestoreScrollPosition(scrollableDiv);
 }
}
 
function ResetScrollPosition(scrollableDiv) {
 if (null != scrollableDiv) {
  // set position
  scrollableDiv.scrollTop = 0;
 }
 document.cookie = "yScrollPos=0";
}
 
function SaveScrollPosition(scrollableDiv) {
 if (null != scrollableDiv) {
  // save the scroll position
  document.cookie = "yScrollPos=" + scrollableDiv.scrollTop;
 }
}
 
function RestoreScrollPosition(scrollableDiv) {
 if (null != scrollableDiv) {
  // get scroll position
  var yScrollPosition = GetCookie("yScrollPos");
  if (yScrollPosition) {
   // restore scroll position
   scrollableDiv.scrollTop = yScrollPosition;
  }
  else {
   // reset scroll position
   scrollableDiv.scrollTop = 0;
  }
 }
}
 
function GetCookie(cookieName) {
 // matches 4 patterns
 // 0th: entire pattern of "cookieName=value;"
 // 1st: beginning of line, or "; " cookie separator--does not capture, as indicated by "?:"
 // 2nd: "(.*?)" -- capture a non-greedy wildcard match
 // 3rd: ";" or end of line--does not capture, as indicated by "?:"
 var patternString = "(?:^|;)\\s*" + cookieName + "=(.*?)(?:;|$)";
 var caseInsensitive = "i";
 var pattern = new RegExp(patternString, caseInsensitive);
 var matches = document.cookie.match(pattern);
 var value = null;
 if ((matches) && (2 == matches.length)) {
  value = matches[1];
 }
 return value;
}

Where to begin? My mother told me, "If you can't say anything nice, don't say anything at all." so maybe it has some merits. Lets see:

• It is in it's own .js file so it can be reused.
• There is some, though mostly useless, comments that document whats going on
• Surprisingly, it does work

Yeah that is about it. I can't really come up with anything else. The fact that I can find anything redeeming about this code is surely a testament for my canonization. Now that I've said something good lets back the cart up and run it over.

I'll just throw out a few of the quick objections and improvements:

• There is no need to check if this is a postback. The objective is to only do this on postback so we should only be calling this method when we are beginning a postback
• The repeated null checks are not necessary and can be pulled out to a higher level
• The removed null checks collapse those functions into 1 line functions that can be pulled out to a higher level
• The reset scroll position is not necessary at all, in fact we specifically never have a need to reset the position because again, this is only supposed to happen during postbacks.

So before we even dig into anything we can distill the mess down to only the required parts:

function SaveScrollPosition(scrollableDiv) {
  document.cookie = "yScrollPos=" + scrollableDiv.scrollTop;
}
function RestoreScrollPosition(scrollableDiv) {
  var yScrollPosition = GetCookie("yScrollPos");
  if (yScrollPosition) {
   scrollableDiv.scrollTop = yScrollPosition;
  }
}
 
function GetCookie(cookieName) {
 // matches 4 patterns
 // 0th: entire pattern of "cookieName=value;"
 // 1st: beginning of line, or "; " cookie separator--does not capture, as indicated by "?:"
 // 2nd: "(.*?)" -- capture a non-greedy wildcard match
 // 3rd: ";" or end of line--does not capture, as indicated by "?:"
 var patternString = "(?:^|;)\\s*" + cookieName + "=(.*?)(?:;|$)";
 var caseInsensitive = "i";
 var pattern = new RegExp(patternString, caseInsensitive);
 var matches = document.cookie.match(pattern);
 var value = null;
 if ((matches) && (2 == matches.length)) {
  value = matches[1];
 }
 return value;
}

Whats left of the code is smaller and more concise. Still ugly, but more manageable. The meat of this approach is based around cookies and regular expressions.

Regular expressions are expressive and powerful, but they are a dual edged sword. They can also be hard to read, hard to build and even harder to debug. When I find myself having to resort to a regular expression to solve a problem, I back up several steps and try to figure out f they are really necessary. One must always carefully weigh the amount of obfuscation brought with complicated regular expressions against clarity and the ability to maintain and debug code.

In the case of the GetCookie function we find a function that is hard to deduce for most programmers. The coder placed some comments, but the comments do not explain what is going on. The comments note that the regular expression matches 4 patterns. Which lead me to more questions:

• Why are there 4 matches?
• What do the 4 matches mean?
• Do all the matches mean the same thing?
• If the expression has 4 matches why do we only look for there to be match length of 2?
• Why do we only ever take the second match?

When I asked the coder these questions, the coder had no real answers. In fact the coder was unsure about anything that was going on with this bit of code. The truly scary part of this is that the coder is in the 5-10 years experience of web programming. For me, under these circumstances, this goes further than code smell. This is not a learning experience or even one of those "Oops" moments we all have. If this is something that you have not "learned" over the years, then what else is going on in your office? Do we need to have a talk about this type of code and why its bad? What purpose will it serve; will you learn and remember next time?

Perhaps a better implementation might have been one where we wired into the framework and and save the position to local memory instead of to a cookie. I might also get a bit more clever and namespace my methods as well as allowing multiple elements to have their positions remembered on the page.

Submitted, hopefully for your approval:

var PostbackActions = PostbackActions || [];
PostbackActions.ScrollPosition = PostbackActions.ScrollPosition || {};
PostbackActions.ScrollPosition.TargetElements = {};
PostbackActions.ScrollPosition.TargetElementPositions = {};
PostbackActions.ScrollPosition.AddTargetElement = function (targetElementId) {
 var element = document.getElementById(targetElementId);
 this.TargetElements.push(element);
};
PostbackActions.ScrollPosition.RemoveTargetElement = function (targetElementId) {
 //remove the element from the list and return the element
};
PostbackActions.ScrollPosition.SaveTargetPositions = function () {
 for (var i = 0; i < this.TargetElements.length; i++) {
  this.TargetElementPositions.push(this.TargetElements[i].scrollTop);
 }
};
PostbackActions.ScrollPosition.RestoreTargetPositions = function () {
 //use shift to remove elements and restore positions
}; 
 
if (typeof (Sys) !== 'undefined') {
 var sysInstance = Sys.WebForms.PageRequestManager.getInstance();
 sysInstance.add_beginRequest(PostbackActions.ScrollPosition.SaveTargetPositions);
 sysInstance.add_endRequest(PostbackActions.ScrollPosition.RestoreTargetPositions);
}

Parenthesis, just for emphasis

Working in a certain language for a number of years has its benefits.  The longer you work with a language the more familiar you become with the nuances and syntax.  Less time is spent on Google learning about the libraries or why one method of doing something is more efficient than another.

I have been working for the past few years doing ASP.NET development with C# as the language of choice.  While I do plenty of C# programming, I also write a lot of HTML and JavaScript.  C# is a strongly typed language while JavaScript is not.  C# is compiled while JavaScript is interpreted.

Over the years I have been careful to keep these things in mind as I program because subtle programming errors that are usually prevented because a language is strongly typed or because the compiler caught me being stupid can cause nightmares and wreak havoc in those languages that are not.

A good example of this is when assignment was used where equality was desired.

In C# we have a compiler that will slap us on the hands for this:

public static void Main(){
 int i = 10; 
 int b = 20;

 if(a = b) //compiler warning
 {
  //assignment always happens
 }
 else
 {
  //dead code
 }
}

But, in JavaScript we don't have a compiler to nag at us for this:

function Main(){
 var i = 10; 
 var b = 20;

 if(a = b) //no compiler so, no warning
 {
  //assignment always happens
 }
 else
 {
  //dead code
 }
};

Well, recently I was bit by just this very type of issue with a twist.  I had some code that was using an integer flag to represent the valid values of an instance variable (see Flag Word).  In my JavaScript code I had de-serialized a value from the server that was supposed to represent the style of an HTML element.  The possible values were:

• Bold
• Underline
• Italic

I had an enumeration that emulated the enumeration on the server and all that was needed was to determine if I should apply the styles to the element.  Easy enough, I just needed to make a bit wise comparison and check the result (see Querying the status of a bit).  So off I went.

function ApplyStyles() {
 var deserializedValue = 7;

 var styleMask = {
  Bold: 1,
  Underline: 2,
  Italic: 4
 }

 if (deserializedValue & styleMask.Underline == styleMask.Underline) {
  //execute because 7 & 2 == 2 should evaluate to true
 }
}

But, the if block never got executed.  Why not?  I set a break point because I figured the values that were being returned from the server were not what I was expecting.  What I found was exactly what I expected.  The values in the watch window looked like the comment in the above code.

So, I called a co-worker over.  He looked at the watch window and scratched his head.  After a bit of poking around in the debugger he asked me to evaluate both sides of the bit-wise and operator (&).  the left side was 7 and the right side was True.  Doh! we added an extra set of parentheses and it worked.

function ApplyStyles() {
 var deserializedValue = 7;

 var styleMask = {
  Bold: 1,
  Underline: 2,
  Italic: 4
 }

 if ((deserializedValue & styleMask.Underline) == styleMask.Underline) {
  //extra parentheses (7 & 2) == 2 NOW its true
 }
}

Why, what happened? "I do this all the time in my C# and don't need the parentheses... why do I need them here." I ranted.  So to prove it to myself,  off I went and looked at my C# code. OMG there they were, an extra set of parentheses . Why!!! Whats up with that?

We need to go back to our intro Computer Science classes and remember operator precedence (see Order of operations - Programming languages).  The bit-wise AND and bit-wise OR operators are of lower operator precedence than equality.  What the hell, I thought they were evaluated before equality, somewhere around the bit-wise shift operators.

Well here is where working in a strongly typed language and relying on the debugger has made me lazy and forgetful.  In C# the compiler nags when you attempt to do this.  You will get a compiler warning.

Operator '&' cannot be applied to operands of type 'int' and 'bool' 

It all makes perfect sense, I have conditioned myself when doing this type of operation.  I have conditioned myself poorly.  I have waited until the compiler identified my bad code and then I would go back and correct it instead of writing better code to being with.

Shame on me!

Pop, push, shift and unshift... really?

Today at work a problem presented itself and the solution gave me pause. I works and the concepts are not foreign to me, but still it feels dirty.  I even asked co-worker, what his approach to this problem was and he had the same first approach as I did.

Lets start with a review:

Just so we are all on the same page lets review some of the data structures we will be discussing.

Stack - a stack represents a last in first out (LIFO) data structure that uses the methods push() and pop() to add and remove elements from the structure.

Queue - a queue represents a first in first out (FIFO) data structure that uses the methods enqueue() and dequeue() to add and remove elements from the structure.

The task:

A dialog has an ordered list of check boxes. When the dialog is shown the state of the check boxes should be preserved such that if the user clicks the cancel button the list will revert to its previous state discarding any changes the user has made.

This task should be done in JavaScript to avoid going to the server since no change was made. The approach: I need to be able to add items to a list. Done!

But, I am too clever for my own good. Instead of just adding the items to a list and iterating over the collection with a for loop, in a split second I decided the best approach would be to use a queue.
I know I can't use a stack because the elements in the stack come out in reverse order. That solution would work but that would require me to iterate the check boxes from the end to the beginning instead of from beginning to end when it came time to restoring their state.

That is ugly, unnecessary and might cause maintainability issues when we have to go back and look at the code in the future. Using a queue would let me add elements (enqueue) and when it came time to removing them I could just take them off (dequeue) in order. I could continue to dequeue until there were no more elements. Brilliant!

But wait... The problem: JavaScript does not support the queue as a data type. However, JavaScript supports the array data type and both the queue and stack can be simulated using methods already defined on the array object, namely:

• pop() - Removes the last element of an array, returns that element
• push() - Adds new elements to the end of an array, returns the new length
• shift() - Removes the first element of an array, returns that element
• unshift() - Adds new elements to the beginning of an array, returns the new length

The realization:

Wait, there it is did you catch it? I've been talking about these data structures and the methods used to control them pop() and push(), enqueue() and dequeue(). Then along came shift() and unshift(). You might already know that what is represented by this collection of functions is known as a double-ended queue or a deque (pronounced deck).

Deque -a deque represents a data structure where elements can be added to or removed from either the front or back.

Really the stack and queue can be considered specializations of deques, and can be implemented using deques.

The gripe:

So up to this point I've only really only talked about some data structures and there seems to be no real problem here right?

Well lets examine the following code:

var checkMarks = [];
  var intialCheckStates = [];

  //Save the initial state
  for (var i = 0; i < checkMarks.length; i++) {
   intialCheckStates.push(checkMarks[i]);
  }

  //later on restore the state
  for (var i = 0; i < intialCheckStates.length; i++) {
   var check = checkMarks.shift();//remove the first element
  }

The nomenclature here is what kills me. It feels dirty and unclean because the terminology shift has connotation to a programmer. The shift operator operates on the binary representation of an integer. The digits are moved, or shifted, to the left or right (see Bit Shifts).

I can understand and visualize why the name shift might be used, after all, the elements of the array are in fact being shifted. I even looked up how different languages name these functions (see Operations).

Ada, C++, Python, and Java get it right. Even PHP had the sense to call the operations array_shift and array_unshift. I think it helps in the confusion. So lets just call a duck a duck. What we really have is an array that supports arbitrary inserts and removal. We don't really have any other data structure. Just a pig in makeup.

The solution:

So, what is the right answer? Well the previous code block works and does the job just fine. I added a comment so as to not confuse my co-workers when they have to maintain this code. So I should just move happily on my way. Right?

Well, for now that is the way I chose to go. However the future solution for this block of code will be actually implement the data structure with the appropriate methods. The following code properly encapsulates the Queue data structure and makes sense to me.

function Queue() {
   this.Elements = [];
   this.Count = 0;
  };
  Queue.prototype.Enqueue = function (element) {
   this.Elements.push(element);
   this.Count++;
  };
  Queue.prototype.Dequeue = function () {
   var element = this.Elements.shift();
   this.Count--;
   return element;
  };
  Queue.prototype.Peek = function () {
   return this.Elements[this.Elements.length];
  };
  Queue.prototype.Count = function () {
   return this.Count;
  };

var checkMarks = [];
  var intialCheckStates = new Queue();

  //Save the initial state
  for (var i = 0; i < checkMarks.length; i++) {
   intialCheckStates .Enqueue(checkMarks[i]);
  }

  //later on restore the state
  while (intialCheckStates.Count > 0) {
   var check = intialCheckStates .Dequeue();
  }

Of course, I find issues with this implementation as well:

• How will we maintain this, inline or in its own source file? 
• Does this make the code any easier to read or does it just bloat the code with a class that is just a wrapper around a single function because I don't like the method's name.

Alas, such is the existence of a persnickety programmer.