test(smokehouse): +/- operator #7343
Conversation
connorjclark
requested review from
brendankenny and
patrickhulce
as code owners
| const NUMBER_REGEXP = /(?:\d|\.)+/.source; | ||
| const OPS_REGEXP = /<=?|>=?|\+\/-/.source; | ||
| const NUMERICAL_EXPECTATION_REGEXP = | ||
| new RegExp(`^(${NUMBER_REGEXP})?\\s?(${OPS_REGEXP})\\s?(${NUMBER_REGEXP})$`); |
There was a problem hiding this comment.
is this clearer than ^((?:\d|\.)+)?\s?(<=?|>=?|\+\/-)\s?((?:\d|\.)+)$
There was a problem hiding this comment.
yes, but I'm not sure I'd call either of them super clear ;)
maybe we can at least get a comment here explaining what's going on in english?
An optional number, single optional whitespace character, an operator, single optional whitespace character, a number.
| const operator = parts[1]; | ||
| const number = parseFloat(parts[2]); | ||
| const operator = parts[2]; | ||
| const numbers = [parts[1], parts[3]].filter(p => typeof p !== 'undefined').map(parseFloat); |
There was a problem hiding this comment.
I think it'd make more sense if you nixed the filter and made it explicit
const [prefixNumber, operator, postfixNumber] = parts
// optional named descriptor, though IMO probably unnecessary and might just add cognitive overload
const targetNumber = typeof prefixNumber === 'undefined' ? postfixNumber : prefixNumberThere was a problem hiding this comment.
done. relying on implicit string -> float conversion. doesn't bother me here.
There was a problem hiding this comment.
I forgot about the parseFloat bit for a sec 😆
maybe my "optional named descriptor" is more important to me than I realized... haha
There was a problem hiding this comment.
ha, it would probably have been simpler to just move off the switch statement :P
| items: [ | ||
| { | ||
| url: 'http://localhost:10200/byte-efficiency/script.js', | ||
| wastedBytes: '46481 +/- 100', |
split from #7065