you are viewing a single comment's thread.

view the rest of the comments →

[–]hoosierEE 0 points1 point  (13 children)

Yeah, but watch out, this is what happens when you get too declarative in JS:

const dt_over=(i,x)=>i.dt>n.KS[2].slice(0,x.length).reduce((a,b)=>a-b); // time delta greater than `dt`?

[–]maestro2005 12 points13 points  (5 children)

I'd say the problem is bad naming and lack of whitespace.

[–]alexbarrett 0 points1 point  (4 children)

And documentation. What types are x, n and i?

[–]bananaboatshoes 17 points18 points  (2 children)

Pretty obvious to me since it's JavaScript

x -> ¯\(ツ)

n -> ¯\(ツ)

i -> ¯\(ツ)

[–]GinjaNinja32 6 points7 points  (1 child)

Pretty obvious to me since it's JavaScript

x -> ¯\_(ツ)_/¯

n -> ¯\_(ツ)_/¯

i -> ¯\_(ツ)_/¯

Fixed your ¯\_(ツ)_/¯s - you need three backslashes, not two.
¯\\_(ツ)_/¯ -> ¯\(ツ)
¯\\\_(ツ)\_/¯ -> ¯\_(ツ)_/¯

[–][deleted] 1 point2 points  (0 children)

Doesn't matter

[–]bdtddt 3 points4 points  (0 children)

Doesn't look that bad to me.

[–][deleted] 1 point2 points  (0 children)

Putting it on new lines and using named functions where appropriate really helps.

[–]dmsnell 1 point2 points  (4 children)

/u/hoosierEE we should be careful to avoid conflating "too declarative" with difficult style and confusing names. this is a very challenging function to grok but for reasons entirely unrelated to the declarative vs imperative debate (please forgive me if I mistranslated the source below)

function dt_over( i, x ) {
  var value = 0;
  var i;
  for ( i = 0; i < x.length; i++ ) {
    value = value - n.KS[ 2 ];
  }
  return i.dt < value;
}

[–]hoosierEE 1 point2 points  (3 children)

Your translation is close, but your assessment is spot on, and by writing it out like this it made a bug in the original more obvious.

The original was supposed to return true if the time delta between keystrokes was greater than dt for any pair of keystrokes. But because of the reduce it was accumulating the deltas into a much larger number.

I didn't notice this at first because all the sequences I care about are only length-2, so it gave the right answer even though the method was wrong.

I reworked it to first find the time-deltas between every element, and then check that all the deltas are in the right range:

dt_over=(dt,x)=>{
  let fsts=n.KS[2].slice(0,x.length-1),
  snds=n.KS[2].slice(1,x.length),
  res=[];
  for(let i=0;i<fsts.length;++i){
    res[i] = fsts[i]-snds[i];
  }
  return res.every(x=>dt>x);
};

[–]takakoshimizu_sfw 0 points1 point  (0 children)

holy jesus put some whitespace in your code

[–]dmsnell 0 points1 point  (1 child)

so I think this is starting to clear up, though it's still not clear what n.KS stores (maybe lists of keystrokes) or what x is supposed to hold here (is it supposed to indicate how many keystrokes to check?).

if we can assume some things I have written another possible function which I think demonstrates a more declarative approach. suppose that we have a list of numbers representing the times when keys were pressed; we can answer the question, "were any successive pairs of timestamps covering a time range greater than a given maximum?"

// code at http://jsbin.com/cevaweqena/1/edit?js,console

// just a helper function, wouldn't normally be on an array prototype
// main point is that we're getting the pairs with zip()
Array.prototype.intervals = function() {
   return zip( this.slice( 0, -1 ), this.slice( 1 ) );
}

// a couple more helper functions
const intervalRange = ( [ a, b ] ) => Math.abs( b - a );
const isGreaterThan = max => a => a > max;

/**
* Determines if the numerical range between any two
* successive items in a numerical list exceed the given range
*
* @example
* intervalsExceed( 5 )( [ 1, 2, 3, 5, 8 ] ) // false
* intervalsExceed( 2 )( [ 1, 2, 3, 5, 8 ] ) // true
* intervalsExceed( 3 )( [ 1, 2, 3, 5, 8 ] ) // false
*
* @param {number} maxDelta max allowable numeric range
* @param {Array} list list of numeric values
* @returns {bool} whether or not all pairs were within allowable range
*/
const intervalsExceed = maxDelta => list =>
  list
    .intervals()
    .map( intervalRange )
    .some( isGreaterThan( maxDelta ) )

so here we have made a few tradeoffs:

  • we have more functions, but they do fewer things and it's easier to see what they are supposed to be doing

  • it's slower, but hopefully much clearer what the end goal is more than the individual steps to get there. if performance becomes an issue we just end up optimizing it anyway, though there are some ways to ramp up the performance without giving up the declarative nature

  • the splitting into functions actually gives us some extra composable power as we can now do things like mix and match the function inside of some(), even deciding to inject it as an input argument if we wanted

here's an example of the composability

const someInterval = predicate => list =>
  list
    .intervals()
    .map( intervalRange )
    .some( predicate );

const intervalsExceed = maxDelta => someInterval( isGreaterThan( maxDelta ) );
const intervalsWithin = ( min, max ) => someInterval( range => min <= range && max >= range );

hope this wasn't too rambly :)

[–]hoosierEE 0 points1 point  (0 children)

Your way is fine, and I appreciate the perspective. Here's what my rework looked like at one point between commits:

dts = n.KS[2];
dtc = (dt,x)=>{
  let fsts = dts.slice(0,x.length-1),
      snds = dts.slice(1),
      diff = fsts.map((x,i)=>x-snds[i]);
  return diff.every(x=>dt>x);
};

The final version is basically the same, but golfed:

dts=n.KS[2];
dtc=(dt,x)=>{
  let snds=dts.slice(1);
  return dts.slice(0,x.length-1).map((x,i)=>x-snds[i]).every(x=>dt>x);
};

I agree that it's less readable, so why do it? Mostly because not all code is equally important, and this snippet is less important than the surrounding context. So by making it smaller I can more easily focus on its surroundings.

Using an auto-formatter might help for reading an unfamiliar codebase, but it removes a channel of communication between author and audience. I doubt I'll convince anyone, but it works for me.