Skip to content

Code design issues in js-numbers.js #1838

@ds26gte

Description

@ds26gte
  • makeNumericBinop takes two function args onFixnums and onBoxnums. It is used in the definitions of 9 functions: add, subtract, multiply, divide, lessThan, lessThanOrEqual, greaterThan, greaterThanEqual, expt.

    In 8 of these functions (i.e., excepting expt), args are initially immediately checked if they're fixnum and something specific is done. This is the same as what the makeNumericBinop-generated function would have done using onFixnums. If the args are not fixnum, then the makeNumericBinop-generated function is called. The purpose of replicating the fixnum check outside makeNumericBinop appears to be to avoid one function call. If so, it makes sense to open-code the non-fixnum part of the code also directly, and save one more function call.

    Since only expt uses makeNumericBinop fully, the latter is really useless even for avoiding code rewriting. makeNumericBinop only makes sense if it is used more than once, fully.

    The other option is to use makeNumericBinop fully in all 9 cases. The open-coding in 8 of the cases seems questionable.

    If we're retaining makeNumericBinop, with the open-coding, we should at least replace the OnFixnums arg for add, subtract, multiply, divide with undefined, as it will never be used. This is already done for the {less,greater}Than... functions.

  • Definiton of Biginter.prototype.sqrt is wrapped in an IIFE (immediately invoked function expression). This makes sense only if the function includes a lexical layer. It doesn't.

  • Definitions of bnByteValue bnShortValue bnToByteArray bnMin bnMax bnAnd bnOr bnXor bnAndNot bnNot bnBitCount bnSetBit bnClearBit bnFlipBit bnModInverse seem to be taken as a batch from HAC Chapter 14 along with others, but these are not actually used and can be scrubbed.

  • Use modern JS idioms for anonymous functions. In many cases, function(x, y) {...} should become (x, y) => .... This would go a long way towards making the code more readable!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions