DevExtreme Data Grid - An Advanced Custom Popup Editor - Refactoring

14 December 2016

This post is part of a series describing the process of creating an advanced popup editor for the DevExtreme Data Grid. You can find the introduction to the post series, as well as an overview of all posts, by following this link.


Step 5: Refactoring

For the sample implementations up to this point, I showed most of the configuration logic in the nested control hierarchy that defines the grid with all its sub-elements. Due to the deep nesting, this structure is not the easiest to maintain and I decide to refactor it. Much of the functionality implemented by the nested elements is tied together quite closely, and the current implementation takes advantage of the availability of certain variables throughout the scope, to keep things concise. In a refactored structure, this isn't possible to the same extent - but on the other hand, such a structure shows more clearly what dependencies each individual element has, because dependencies need to be passed in as parameters in many cases. (Of course it would also be technically possible to keep everything in global variables, but I'm not even going to go there - I hope that's not something you would seriously consider!)

I have created a refactored version of my sample, where I extracted the logic into individual functions, passing parameters through as needed. This is only one structural option, there are certainly others! I hope it helps you come up with some techniques if you have large control structures to maintain. Here is the refactored code, below it you can find a number of notes on individual decisions I made while applying structural changes.

Data layer structure

At the top of the code file, the first item I come across is my data layer implementation. So far, this consisted of several types, functions and of course the demo data definition. Even though the data layer implementation would probably look a bit different (and it likely wouldn't live in the same code file as the UI code) in a real application, I decided to change its structure anyway to highlight some approaches and to help with the rest of the refactoring work.

In the refactored code, I create a dataLayer object to encapsulate the data, the types and the helper functions. I use an IIFE to create this object, and all references from the original code that referred to elements that now become part of the new object need to be changed. The purpose of this step is mainly to make the rest of my refactoring work easier: by removing the data layer variables and types from the global scope, I will receive errors when my UI code tries to access these elements directly anywhere.

Functional structures

For each distinct element of my setup, I introduce a separate createXXX function. I write these functions to work only with their input parameters (with few exceptions — see below), so that the parameter lists serve as documentation of the dependencies for each function. All the major event handlers also have their own functions that receive a number of parameters, so I can reuse them with varying wrappers in several places.

Assigning event handlers

The structure of my popup window with all its nested elements is rather complex, and there are several event handlers (for example gconfirmAssignment) that access several controls contained in the hierarchy to do their job. In my old implementation, I was assigning event handlers at various points in the setup hierarchy — sometimes there were specific reasons to assign an event handler in a certain place, sometimes it might just have been convenient at the time. With a clean structure where element creation is encapsulated, I can't assign event handlers in arbitrary places because some context information may not be available at the time. For example, I had a line of code in the old version that assigned the confirmAssignment handler to the list dblclick event, right after the list was created. This is no longer possible now because confirmAssignment requires lots of context information that is not, or not yet, available when the list is created in its own createPopupSupplierList function.

I decided to solve this problem by assigning all relevant events in one central place, at a point in time when the complete control hierarchy has been created. This happens in the onContentReady handler on the dxPopover. I chose to leave the code in that place instead of extracting it again, since I judged the showPopup function not to be overly complex. Extracting the event handler assignment code is easy, in case I change my mind about this later.

Exceptions to the functional rules

To assign the event handlers, I need access to various elements of the hierarchy. Unfortunately, I found a few cases where these elements are not easily accessible as children of the popup. The top-level form is created by the logic assigned to contentTemplate, but in order to access it later, I need to use the call chain component.content().children().first(). This is not optimal because it assumes a certain structure.

I used an alternative approach for the nested form and list elements, and this technique could also be applied to the top-level form. The nested form and list elements are created by item template functions on dxForm, and there doesn't seem to be an easy way of retrieving these generated elements from the dxForm later on. (Note that the dxForm has a getEditor helper, but that only works with auto-generated editors, not with items generated through a template.) So I decided to assign unique ids to the list and the nested form in their creation functions, and use these ids when I need to access the elements at a later point. I don't see this as a particularly elegant approach because it is technically access to a global variable, and because in more complex scenarios it might not be safe to assume that the ids I use are actually unique.

Eliminating global state

There is one item of global state in my old implementation: the creatingNew flag that is set to true when the user clicks the button in the popup to create a new supplier. My persistence logic in confirmAssignment depends on this, and it still does in my refactored code. The difference is that the flag is now being passed in as a parameter, and the state is technically kept in a closure created during the event handler assignment. The variable creatingNew exists only on this level now, and is accessed exclusively by the two functions created to wrap the event handlers createNewSupplier and confirmAssignment. This is a functional technique that allows, in this case, two disconnected pieces of code to share a piece of state information without publishing that information to be read or even changed from elsewhere in the code.

Encapsulating parameters for createDataGrid

I decided to encapsulate the set of parameters that refer to data layer elements, and the set that refers to event handlers, each in their own object for the signature of the createDataGrid function. This enables me to pass in my existing dataLayer as one parameter, which satisfies the contract expressed by the signature of createDataGrid. Some flexibility is gained this way, but I'll readily admit that the benefit in this code sample is not too impressive.

Results

Overall, my refactoring work has simplified the code structure considerably. There are around 30 lines more code than in the old sample, which seems very reasonable for this much cleaner implementation. Since I made a lot of changes, I'm not completely sure I covered all relevant decisions in this summary — let me know if you'd like me to elaborate on something!

no comments
No Comments

Please login or register to post comments.