4
\$\begingroup\$

Pattern matching for years, I need to be able to return all years from a string.

  1. When there is a dash "-" I need to return all numbers between. For example, '2004-2006' should return an array of numbers [2004,2005,2006];
  2. If the number at the beginning of the dash is larger than the number following it, nothing should be printed out. For example '2006-2005' should return [].
  3. years between commas should simply be returned as long as they do not exceed the current year, and are not less than 1999. For example '1998,2020' should return an empty array [].
  4. There should be no duplicates and the values should be ordered.

I believe my code could be written in a more elegant and consise way.


Here are my test cases:

assert.deepEqual(parseRange('2002-2005, 2002-2005, 2002 - 2005'), [2002, 2003, 2004, 2005]);
assert.deepEqual(parseRange('2017'), [2017]);
assert.deepEqual(parseRange('2017 - 2015'), []);
assert.deepEqual(parseRange('2015 - 2015'), [2015]);
assert.deepEqual(parseRange('1999 , 2000,    , 2008'), [1999, 2000, 2008]);
assert.deepEqual(parseRange('2015, 2014, 2010'), [2010, 2014, 2015]);
assert.deepEqual(parseRange('1999, 3000'), [1999]);
assert.deepEqual(parseRange('1998, 2020'), []);


Here is my working Code

function parseRange(string) {
  return [...new Set(string.split(',').map(range => range.includes('-') ? range.split('-').map(num => +num) : +range)
    .filter(set => Array.isArray(set) ? set[0] <= set[1] : set > 1998 && set <= new Date().getFullYear()).reduceRight((arr, val) =>
      arr.concat(Array.isArray(val) ? Array(val[1] - val[0] + 1).fill().map((_, num) => val[0] + num) : val), []))].sort();
}

console.log(parseRange('2002-2005, 2002-2005, 2002 - 2005'));
console.log(parseRange('2017'));
console.log(parseRange('2017 - 2015'));
console.log(parseRange('2015 - 2015'));
console.log(parseRange('1999 , 2000,    , 2008'));
console.log(parseRange('2015, 2014, 2010'));
console.log(parseRange('1999, 3000'));
console.log(parseRange('1998, 2020'));

\$\endgroup\$
  • 1
    \$\begingroup\$ If you're looking for elegance and conciseness, don't golf the code. Don't sacrifice brevity for readability unless you're doing this for PPCG or something. \$\endgroup\$ – omgimanerd Aug 4 '17 at 21:15
  • \$\begingroup\$ @omgimanerd point taken. but I can read it just fine. Also, it's easier on the fingers to type less code. However, that's why I am here. Send some wisdom my way! \$\endgroup\$ – Rick Aug 4 '17 at 21:32
  • \$\begingroup\$ @omgimanerd sacrifysing brevity for readability is actually totally fine, because the latter one is much more important in a long run. The reverse is quite arguable, though... I think, I'm confused with what you're saying. \$\endgroup\$ – Igor Soloydenko Oct 5 '17 at 6:38
  • \$\begingroup\$ Whoops yeah I meant the reverse. Don't sacrifice readability for brevity. I don't know why I reversed it \$\endgroup\$ – omgimanerd Oct 5 '17 at 13:10
2
\$\begingroup\$

You're asking for elegance and conciseness. Elegance doesn't always mean short/hackish, and neither does conciseness. You should never sacrifice readability for brevity unless you're codegolfing. As per your comment, just because you can read it doesn't mean the next maintainer or developer can. Someone (maybe you) revisiting this in 5 years should know what the code does without having to pick through it. Let's start by breaking apart the code into its components.

function parseRange2(string) {
  var expr = string.split(',').map(range => range.includes('-') ? range.split('-').map(num => +num) : +range)
  expr = expr.filter(set => Array.isArray(set) ? set[0] <= set[1] : set > 1998 && set <= new Date().getFullYear())
  expr = expr.reduceRight((arr, val) =>
      arr.concat(Array.isArray(val) ? Array(val[1] - val[0] + 1).fill().map((_, num) => val[0] + num) : val), [])
  return [...new Set(expr)].sort()
}

This looks a little more readable. However, someone reading this for a production codebase would have to actually figure out what each expression did. Let's rename some variables and make this easier to digest.

function parseRange2(string) {
  // This expression is still kind of pushing it in terms of readability.
  // Split string in ranges and values
  var ranges = string.split(',').map(range => range.includes('-') ? range.split('-').map(num => +num) : +range)
  // Remove invalid ranges
  var validRanges = ranges.filter(set => Array.isArray(set) ? set[0] <= set[1] : set > 1998 && set <= new Date().getFullYear())
  // Expand range arrays
  var expandedRanges = validRanges.reduceRight((arr, val) =>
      arr.concat(Array.isArray(val) ? Array(val[1] - val[0] + 1).fill().map((_, num) => val[0] + num) : val), [])
  // Return sorted and unique years
  return [...new Set(expandedRanges)].sort()
}

Now let's refactor some of this stuff into helper functions.

const getYearRange = (start, end) => {
  return Array(end - start + 1).fill().map((_, num) => start + num)
}

const isValidEntry = entry => {
  return Array.isArray(entry) ? entry[0] <= entry[1] : entry > 1998 && entry <= new Date().getFullYear()
}

const parseRange2 = string => {
  // This expression is still kind of pushing it in terms of readability.
  // Split string in ranges and values
  var ranges = string.split(',').map(range => range.includes('-') ? range.split('-').map(num => +num) : +range)
  // Remove invalid ranges
  var validRanges = ranges.filter(isValidEntry)
  // Expand range arrays
  var expandedRanges = validRanges.reduceRight((arr, val) =>
      arr.concat(Array.isArray(val) ? getYearRange(val[0], val[1]) : val), [])
  // Return sorted and unique years
  return [...new Set(expandedRanges)].sort()
}

You get the point. You should refactor further to make the code easier to maintain and develop.

Now on the other hand, if you're looking to use every JavaScript hack in the book to golf your code, you could use reduce() instead of reduceRight().

For -2 characters, you could use the following to generate your date range:

[...Array(val[1] - val[0] + 1)].map((_, num) => val[0] + num)

as opposed to:

Array(val[1] - val[0] + 1).fill().map((_, num) => val[0] + num)

You can use the truthiness of arr[0] instead of Array.isArray(arr) to check if an object is an array, which yields the following code:

function parseRange2(string) {
  return [...new Set(string.split(',').map(range => range.includes('-') ? range.split('-').map(num => +num) : +range)
    .filter(set => set[0] ? set[0] <= set[1] : set > 1998 && set <= new Date().getFullYear()).reduce((arr, val) =>
      arr.concat(val[0] ? [...Array(val[1] - val[0] + 1)].map((_, num) => val[0] + num) : val), []))].sort();
}

Addendum: Have you tried '1-10, 1999, 2002-2005, 2002-2005, 2002 - 2005, 2010' as a test case?

Result: [ 1, 10, 1999, 2, 2002, 2003, 2004, 2005, 2010, 3, 4, 5, 6, 7, 8, 9 ]

\$\endgroup\$
  • \$\begingroup\$ A+ for effort, but these optimization are not game changers. Can you do it with closures? \$\endgroup\$ – Rick Aug 4 '17 at 23:22
  • \$\begingroup\$ Mmm, I suppose so. I assume by now your goal is short code. I'll see if I can think of any more closure hacks. \$\endgroup\$ – omgimanerd Aug 4 '17 at 23:24
  • \$\begingroup\$ super short, like multi-dimensional short. Like, let's solve this by looking at the problem differently short. An ideal solution would probably involve closures, hashing, nested objects, and array.push. \$\endgroup\$ – Rick Aug 4 '17 at 23:27
  • \$\begingroup\$ Do you care about speed? Some of the hacks are code-golfing hacks that make the code run a bit slower in exchange for being a little shorter. \$\endgroup\$ – omgimanerd Aug 4 '17 at 23:36
  • \$\begingroup\$ umm sure, but from my experience, shorter code is usually faster, unless it's recursion or nested loops. \$\endgroup\$ – Rick Aug 4 '17 at 23:38
0
\$\begingroup\$

I tried to rewrite the code in a more human readable form.

It's usually easier if we first start by unifying the input format (eliminating the edge cases).

In this case, obvious obstacles are whitespaces and empty years (, ,) so let's get rid of them first:

.replace(/\s/g, '').split(',').filter(String)

Then we can benefit from a unified format for range, in this case, I assumed a year without range is just a range that starts and ends with itself ie. 1999 is equivalent to 1999-1999. So I just split the ranges like this:

.map(r => r.split('-'))

Doing this I'll have a list of ranges with this format: [beginning, end] in which end can be empty '1999' -> ['1999'] & '1999-2001' -> ['1999', '2001']

Now it's easier to expand ranges

.reduce((arr, [b, e = b]) => e - b < 0 ? arr : arr.concat([...Array(e - b + 1).keys()].map(i => +b + i)) , [])

Note: b is the beginning and e is the end of the range (I exploited ES6 destructuring assignment to assign b and e from the range array, and default parameters for when e is not given)

Now I have the flat list of the years [1999, 2003, 2003, 2004, ...], the rest is just sorting the array and removing the duplicates.

Final solution:

function parseRange (string) {
    return string
        // pre-processing: remove whitespaces, then generate range array e.g. [['1999'], ['2002', '2005']]
        .replace(/\s/g, '').split(',').filter(String).map(r => r.split('-'))
        // flatten the range array by expanding ranges (['1999'] ~ ['1999', '1999'])
        // result: ['1999', '2002', '2003', '2004', '2005']
        .reduce((arr, [b, e = b]) => e - b < 0 ? arr : arr.concat([...Array(e - b + 1).keys()].map(i => +b + i)) , [])
        // sort the array and remove duplicates
        .sort().filter((v, i, arr) => !i || arr[i - 1] !== v)
        .filter(y => y > 1998 && y <= new Date().getFullYear());
}
\$\endgroup\$

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.