1

Original

export const isTimeStrValid = str => {
  return str.length >= 4 && moment(str, ['H:mm', 'HH:mm'], true).isValid();
};

Ramda

export const isTimeStrValid = R.allPass([
  R.pipe(R.length, R.gte(R.__, 4)),
  R.pipe(
    s => moment(s, ['H:mm', 'HH:mm'], true),
    R.invoker(0, 'isValid'),
  ),
]);

The Ramda/functional programming version feels verbose but I can't quite figure out how to make it more elegant. As it stands, it appears that the original/imperative version is easier to read/understand. Does my Ramda version follow conventions/best practices?

Edmond C
  • 520
  • 1
  • 5
  • 16

1 Answers1

4

Personally I think your original function is fine. Since you're using ES6, you can get rid of the (imperative) return statement:

export const isTimeStrValid = str =>
  str.length >= 4 && moment(str, ['H:mm', 'HH:mm'], true).isValid();

Whether your "functional" version is ok from a best-practice perspective is hard to tell as this is mostly going to be a subjective debate.

The only thing I can tell, is that a point-free style can lead to verbosity, but you can mitigate that by splitting things into smaller chunks:

Would this read better to you for example?

const isTimeStrValid = R.both(isValidString, isValidMoment);

Where isValidString and isValidMoment are reusable functions:

const isValidString = R.compose(R.lte(4), R.length);
const toMoment = R.curry((strict, formats, datestr) => moment(datestr, formats, strict));
const isValidMoment = R.compose(R.invoker(0, 'isValid'), toMoment(true, ['H:mm', 'HH:mm']));
customcommander
  • 17,580
  • 5
  • 58
  • 84
  • 1
    great answer. advice wrt _reusable functions_ is echoed :D – Mulan Mar 06 '19 at 13:25
  • 1
    I concur. Ramda is a library of functions that may be helpful. It should never be a goal. – Scott Sauyet Mar 06 '19 at 15:38
  • 2
    Note that `R.both` is often a reasonable alternative to `R.allPass`. That is, `both(f, g)` reads better to me that `allPass([f, g])`. But they do have slightly different APIs, with `allPass` guaranteeing a boolean and `both` returning false-y or truth-y answers. – Scott Sauyet Mar 06 '19 at 15:42
  • 1
    I agree, `R.both` feels better in this case. I'll update. – customcommander Mar 06 '19 at 17:14
  • I agree w/ @ScottSauyet Ramda doesn't need to be a goal. customcommander's reusable refactor is also a great alternative to point-free. And sometimes a mix of imperative does read better than pure declarative/functional, need to inspect and decide case-by-case. Thanks every one! – Edmond C Mar 07 '19 at 05:55