2

I'm working on a fairly new project where I'd like to share some config items used by a JavaScript plugin. Specifically, the project is using jQuery dataTables and I want to find a good way of sharing config items.

At the moment, the code I've been given is fairly simple. It is roughly equivalent to:

// It might not be super-essential to read this

function TableSuperClass(tableElement, moreOptions) {
    var options = {
         pagination: true,    // turn on pagination
         iDisplayLength : 50, // pages are fifty rows long
         lengthChange: false, // stop users changing length of pages
         searching : false    // disables searching
    };

    jQuery.extend(options, moreOptions); // merge moreOptions into options
    jQuery(tableElement).DataTable(options); // we create a table with the extended options

    // This is a super-handy method we'd like to use everywhere
    this.repopulate = function usefulRepopulateFunction() {...};
}

function ObjectOwningTable() {
    // this object does other things too

    this.table = TableSuperClass({
         // various column mappings for a particular table type and
         // what to order the table on
         columns : [...],
         // implements row highlighting, partly based on column mappings
         fnRowCallback : function() {...} 
    });
}

function AnotherObjectOwningTable() {
    // this object does other things too

    this.table = TableSuperClass({
         // various column mappings for a different type of table, switched
         // based on some logic inside this class
         columns : someTest ? [...] : [...],
         // implements ordering a different way
         aDesc : [...] // aDesc is a legacy API call that lets you specify the col to order on             
    });     

}

The problem is that I now have another sort of table. It is used in a print view. It has the same column mappings as ObjectOwningTable, it needs the super-convenient repopulate method and disabled searching from TableSuperClass, but it doesn't have pagination, as a printout has to show the whole data set.

I can think of two basic solutions:

  1. Solve the problem with inheritance. Make TableSuperClass provides the handy repopulate interface and disabled searching, then a subclass with pagination, then call that in ObjectOwningTable and AnotherObjectOwningTable. For my print table, I call TableSuperClass with the same column mappings as used in ObjectOwningTable (maybe I extract them to a global constant or something).
  2. Solve the problem with composition. I have a single DynamicTable that provides just the repopulate interface, then pass in a list of plain objects that are applied to the options in order. These plain objects all live on an enum somewhere, so I can see how they might clash as I write them, and allow me to mix-and-match functionality further down the line.

So, at the consumer level, the first solution would end up looking like

function ObjectOwningTable() {
    this.table = PagedTable(FOO_REPORT_COLUMN_MAPPINGS);
}

function AnotherObjectOwningTable() {
    this.table = PagedTable(someTest? BAR_COLUMN_MAPPINGS : BAZ_COLUMN_MAPPINGS);
}

function PrintObjectOwningTable() {
    this.table = TableSuperClass(FOO_REPORT_COLUMN_MAPPINGS);
}

Whereas the latter would look like

function ObjectOwningTable() {
    this.table = DynamicTable([
        DATA_TABLES_OPTIONS.pagingFixedAtFiftyRows,
        DATA_TABLES_OPTIONS.disableSearching,
        DATA_TABLES_OPTIONS.fooColumnsAndHighlighting
    ]);
}

function AnotherObjectOwningTable() {
    var columnMappings = someTest ? DATA_TABLES_OPTIONS.barColumnMappings : DATA_TABLES_OPTIONS.bazColumnMappings;
    this.table = DynamicTable([
       DATA_TABLES_OPTIONS.pagingFixedAtFiftyRows,
       DATA_TABLES_OPTIONS.disableSearching,
       columnMappings
    ]);
}

function PrintObjectOwningTable() {
    this.table = DynamicTable([
       DATA_TABLES_OPTIONS.disableSearching,
       DATA_TABLES_OPTIONS.fooColumnsAndHighlighting,
       DATA_TABLES_OPTIONS.disableSortingControls
    ]);
}

I'm not sure what to choose. The inheritance approach feels very problematic. It seems very inflexible, and I can see a lot of ways for future developers to 'trap' functionality in particular class taxonomies. I also think that long inheritance chains are harder to reason about than flat structures. But the compositional approach is a bit more verbose, and I think my co-workers might object to the repetition. Yet I think that mixing and matching is really handy.

There is a third option, which is to create a subclass of DynamicTable that binds the pagingFixedAtFiftyRows and disableSearching options, and is then called by the objects that use it, passing in their peculiar column mappings. But I wonder if this makes things more complicated, using both subclasses and mixins. I also wonder if it creates the temptation for other developers to add functionality to the subclass when they should really try and create mixins.

What would you recommend?

  • 1
    how about having another factory class to generate configuration? it doesn't help with the duplication but at least all the configuration generation is centralized into one place. And personally I feel duplication there is ok – Phuong Nguyen Nov 8 '14 at 12:30
  • @PhuongNguyen - that's a good idea – Jimmy Breck-McKye Nov 8 '14 at 18:04
  • 1
    Javascript made me think about Delegation. Just wanted to mention it as an alternative solution. – User Nov 9 '14 at 13:20
1

Given just the two possibilities you present, I think the first, inheritance, solution is better, even though I generally don't think inheritance solutions are better.

If I understand the code correctly, you second solution involves just one type DynamicTable that has a whole slew of booleans in it to customize its behavior. This means that anytime someone needs a table with different behavior, they will have to go into DynamicTable to add yet another boolean and conditionally change the tables behavior based on the new boolean.

Better, I think, would be to follow the Open/Closed Principle. DynamicTable should employ judicious use of the Strategy pattern such that your various other types can customize its behavior by injecting blocks of code into it, rather than simply setting booleans.

For example, you have the notion of pagingFixedAtFiftyRows. Presumably, you also have the notion of paging not being fixed at fifty rows, and that also opens the door to the idea of paging being fixed, but at something other than fifty rows. Expressing the notion of having pagingFixedAtFIftyRows as a boolean is kind of silly in this regard. At minimum, it should be an integer (where 0 would mean paging is not fixed.)

Another example, you have the notion of disableSearching. Presumably, in the DynamicTable you have at least one if statement that checks this bool and does something different depending on whether or not it is true. Better would be to have a separate Search Module that contains the conditional code. If a user wants searching, they inject the search module into the table. This also opens the door for them to do other interesting things to filter the output of the table that isn't exactly like searching.

Good luck with whatever solution you decide, but I suggest you look into the Strategy Pattern, you will likely get more ideas http://en.wikipedia.org/wiki/Strategy_pattern

  • No, that's not quite how the table stuff works. I'm already passing a set of config options to a plugin, so there's no logic inside the proposed DynamicTable. I suppose what I'm asking is how to share and pass around those sets of config flags, if indeed I need to (I think I do, as there are a few cases where multiple options need to be set in order to achieve a particular effect - for instance, highlighting rows is done by making one column in the data set invisible, and then running a callback that adds a class based on that column's data). – Jimmy Breck-McKye Nov 8 '14 at 17:32
  • Maybe I could separate out the config flags from the DynamicTable wrapper, and just have a base config for the report tables and a base config for print tables, with a little repetition between them. Maybe that would make things simpler? I don't know. – Jimmy Breck-McKye Nov 8 '14 at 18:41
  • "Classes in an OO design that do not add meaningful behavior should be eliminated as irrelevant." -- Arthur Riel PrintObjectOwningTable et al. don't seem to be adding meaningful behavior, rather they just seem to be some sort of configuration buckets for DynamicTable. Maybe there is stuff you aren't showing, but as it stands, I think that either behavior needs to be moved into these classes, or they need to go away. – Daniel T. Nov 8 '14 at 19:20
  • Well, the consumer classes are creating objects that do many things beyond drawing tables. They own the tables, but they also own other views, and pass models to them for display. So the question becomes, I suppose, how should I handle these groups of config items used in these different cases? Does that make sense? – Jimmy Breck-McKye Nov 8 '14 at 20:01
  • Essentially: classes X and Y both own an instance of R compositionally. R is calling T and passing config options. X and Y are also passing config options for R to itself pass to T. I want P to pass X's config options to T, and for P to get a method of R, but I need to override a config option that R is passing to T. – Jimmy Breck-McKye Nov 8 '14 at 20:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.