Multi-field sorting

Question

A function to sort data on multiple fields.

I welcome feedback.

const flatten = o => Object.values(o).reduce((acc, c) => (Array.isArray(c) ? [...acc, ...c] : typeof c === 'object' ? [...acc, ...flatten(c)] : [...acc, c]), []);
const flow = (...fns) => data => fns.reduce((acc, c) => c(acc), data);
const GROUP = Symbol('group');
const asGroup = (result = []) => ((result[GROUP] = true), result);
const isGroup = o => o[GROUP];

const groupBy = field => (data, key) =>
    data.reduce((acc, c) => 
        ((key = c[field]), (acc[key] ? 
            (acc[key].push(c), acc) : 
                ((acc[key] = asGroup([c])), acc))), {});

const recurse = (test) => (transform) => o =>
    test(o)
        ? transform(o)
        : Object.entries(o)
            .sort(([_,v1],[__,v2]) => v1 - v2)
            .reduce((acc, [k, v]) => (test(v) ? 
                  ((acc[k] = transform(v)), acc) : 
                  ((acc[k] = recurse(test)(transform)(v)), acc)), {});
                  
const group = (...fields) => flow(...fields.map(flow(groupBy, recurse(isGroup))), flatten);

const rows = asGroup([{
        id: 0,
        steamid: '2',
        website: 'a'
    }, {
        id: 1,
        steamid: '2',
        website: 'b'
    }, {
        id: 2,
        steamid: '2',
        website: 'a'
    }, {
        id: 3,
        steamid: '1',
        website: 'b'
    }, {
        id: 4,
        steamid: '0',
        website: 'b'
    }, {
        id: 5,
        steamid: '2',
        website: 'b'
    }
]);
console.log(JSON.stringify(group('steamid', 'website', 'id')(rows), null, 2));

Description:

This works by creating a recursive groupBy grouping function for each of the fields supplied as an argument.

These dynamically created groupBy functions are then invoked one by one, passing the result between, starting with the supplied data.

Each groupBy function instance creates an object and adds properties to it corresponding to the key values for the field being grouped.

By calling these groupBy functions successively, we create a progressively more nested tree of objects, with groups at each successive level marked as being groups using a symbol.

The final result is a tree of objects, with keys corresponding to the field used for indexing at that level.

Finally, we flatten the tree and the final order is visible.

Answer 1

Disclaimer: I totally understand that my answer is very subjective, but you may still find it useful. I am myself a big proponent of functional programming, and from this perspective your code looks decent. It uses abstractions efficiently, and is DRY.

Functional Programming is Great; Too Many Abstractions Is Questionable

However, I'm also a big proponent of clean code. And in my very humble opinion, this code is very hard to understand (especially to anyone who is new to functional programming) because it is almost overly generalized.

The code is dense -- that's cool, but not everyone can deal with so many levels of abstraction at once.

Hard Things in Computer Science: Naming Things

Unfortunately, I am unable to quickly find any abstractions that you can throw away. So there are a few other things we can look at.

The naming. It suffers. Yes, I know we see the examples of home brewed composition functions with fns all over the place. I believe it's a bad practice and we should do better. It takes just a few keystrokes in your editor/IDE to rename all occurrences of a symbol.

Similarly, the following names are just as bad: o, c, data, field, acc (in my real life example a developer once confused accumulator with account, not kidding). I strongly recommend giving these entities much more context specific names. While things like i and j are commonly used and well-known, I still find them horrible: a) they unnecessarily complicate things, even a small "brain cycle" to interpret it is a distraction; b) they often cause OBOE; c) devs moved to forEach() for a reason. Last but not the list, many devs hold an opinion that one letter long variable name is a smell that suggests that the entity has no strong reason to exist.

More Functions?

Yes, over time developers working with JavaScript get used to structures like const x = a => b => c => { ... }. Still, IMO each of this levels add extra "brain cycles" needed to understand what's available in which scope; or (more importantly?) what's not available.

I think, that proper indentation or function extraction or both may help reducing the complexity here. Probably, some function will exist just to split a huge one into a set of smaller ones. (Otherwise, we could just rewrite group() inlining all the other functions, which would make the result a HUGE god-function -- while still functional, very hard to deal with).

---

I tried to rename things and format it differently as per my understanding of readable code. I'm still struggling with a few names as you can see, but this is just to demonstrate the ideas in actions, anyway...

First, flatten and compose.

const flatten = objectToFlatten => Object
  .values(objectToFlatten)
  .reduce(
    (subResultArray, currentElement) =>
      Array.isArray(currentElement) ?
        [...subResultArray, ...currentElement] :
        typeof currentElement === 'object' ?
          [...subResultArray, ...flatten(currentElement)] :
          [...subResultArray, currentElement],
    []
  );

const compose = (...functionList) =>
  argument =>
    functionList.reduce((previousResult, currentFunction) => currentFunction(previousResult), argument);

Group logic related "helper" functions.

const GROUP = Symbol('group');
const asGroup = (result = []) => ((result[GROUP] = true), result);
const isGroup = object => object[GROUP];

const groupBy = groupingField =>
  (objectList, key) =>
    objectList.reduce(
      (subresultObject, dataElement) => (
        (key = dataElement[groupingField]),
        (
          subresultObject[key] ?
            (subresultObject[key].push(dataElement), subresultObject) :
            ((subresultObject[key] = asGroup([dataElement])), subresultObject)
        )
      ),
      {}
    );

The recursive transformation working horse.

const transformRecursivelyWithPredicate = predicate =>
  transform =>
    objectToTransform =>
      predicate(objectToTransform)
        ? transform(objectToTransform)
        : Object.entries(objectToTransform)
          .sort(([_, value1], [__, value2]) => value1 - value2)
          .reduce(
            (subresult, [key, value]) => (
              predicate(value) ? 
                ((subresult[key] = transform(value)), subresult) : 
                ((subresult[key] = transformRecursivelyWithPredicate(predicate)(transform)(value)), subresult)
            ),
            {}
          );

And the original composed beast. :)

const group = (...groupingFields) =>
  compose(...groupingFields.map(compose(groupBy, transformRecursivelyWithPredicate(isGroup))), flatten);

---

Little Warning

The very last thing to say is that there's a concerning piece of code. Not a bug per se. I'd try to treat it as a warning.

Original code:

Same, in my refactored code:

I'm pretty sure you already see what I'm going to say. It's not a great thing to reassign a value that was provided into the arrow function by the caller. Today your code works. Tomorrow a junior developer touches it and things crash...

I'd recommend to rewrite (subresultObject, dataElement) => ... as (subresultObject, _) => ... or even as subresultObject => ... and use a locally scoped const "variable" to prevent bugs.

---

P.S. I apologize in advance for using strong words and for my way of thinking potentially misaligning with the coding standards of your organization. I'm not questioning your internal agreements of course, but rather trying to analyse and evaluate the code as a "spherical cow in a vacuum" from a perspective of the developer on the Clapham omnibus (who obviously can not jump as high as you can).

P.P.S. This is the great question and the code to read/think about!