4
\$\begingroup\$

I have posted this in continuation to my earlier post Quadratic equation solver in JavaScript.

Tried to improve on the shortcomings last time and also changed the logic. It is still not covering all the input cases. Planning to improve it over time with feedbacks.

Recently started learning about Functional Programming and would also want this program to be reviewed for correct use of Functional Programming techniques.

Example:

const solutions = solveEquation('2 * x^2 - 10 * x + 12');
console.log(solutions); // [2, 3]

Solution:

/**
 *  Solves a Quadratic/Linear Equation Equation 
 *  @param {string} equation 
 *  Input variants of equation:
 *  ax^2 + bx + c,
 *  ax^2 + bx,
 *  ax^2 + c,
 *  bx + c,
 *  c,
 *  For a = ax^2, +ax^2, -ax^2, a*x^2, axx, a*x*x, ax*x, x^2, +x^2, -x^2, 1/2 x^2 but not (1 1/2 x^2 as trimming spaces)
 *  For b = bx, +bx, -bx, b*x, x, +x, -x
 *  For c = c, -c, +c, 0
 *  Where a,b,c are Integers and Floats
 *  Spaces are removed
 *  TBD for cases:
 *  bx + ax^2 + c,
 *  (1 * x ^ 2 + 1 * x ^ 2) === ( 2 * x ^ 2)
 *  Throw valid Errors
 *  @returns An array of roots, empty array if no roots
 */
function solveQuadraticEquation(equation) {
    // # Functional Programming
    // # Higher Order Functions
    // # Pure Functions
    // # No Side Effects
    try {
        // In Functional Programming is function chaining like this correct.
        return getRoots(getCoefficients(removeStar(removeSpaces(equation))));
    }
    catch (e) {
        console.log(e);
    }
}
/** 
 * Removes ' ' from a string
 * @param {string} equation
 * @returns string without spaces ' '
 */
function removeSpaces(equation) {
    const _equation = equation;
    return _equation.replace(/\s/g, '');
}
/**
 * Removes '*' from a string
 * @param {string} equation
 * @returns string without stars '*'
 */
function removeStar(equation) {
    const _equation = equation;
    return _equation.replace(/\*/g, '');
}
/**
 * Parse equation to find Coefficients
 * @param {any} equation 
 * @returns Array of (Integers/Floats) Quadraric Equation Coefficients. 
 */
function getCoefficients(equation) {
    // Used eval instead of parseFloat to support Fractions
    const _equation = equation;
    const regxForA =
        (_equation.search(/[0-9]x\^2/g) !== -1)
            ? /x\^2/g
            : (_equation.search(/x\^2/g) !== -1)
                ? /x\^2/g
                : (_equation.search(/[0-9]xx/g) !== -1)
                    ? /xx/g
                    : (_equation.search(/xx/g) !== -1)
                        ? /xx/g
                        : "ERROR"
    const a = eval(getMatchingValue(_equation, regxForA));
    const equationForB =
        (regxForA === 'ERROR')
            ? _equation
            : _equation.split(getRegexSource(regxForA))[1];
    const regxForB =
        (equationForB.search(/[0-9]x/g) !== -1)
            ? /x/g
            : (equationForB.search(/x/g) !== -1)
                ? /x/g
                : "ERROR";
    const b = (regxForB === "ERROR") ? 0 : eval(getMatchingValue(equationForB, regxForB));
    const c = (b === 0)
        ? eval(equationForB)
        : !isNaN(eval(equationForB.split(getRegexSource(regxForB))[1]))
            ? eval(equationForB.split(getRegexSource(regxForB))[1])
            : 0;
    return [a, b, c];
}
/**
 * Get Formatted Regex Source
 * @param {any} regex 
 * @returns Source of a regex after removing any \
 */
function getRegexSource(regex) {
    const _regex = regex;
    return (_regex.source).toString().replace(/\\/g, '');
}
/**
 * Iterates over equation to find coffecient
 * @param {any} equation 
 * @param {any} regx 
 * @returns (Integers/Floats) coffecient
 */
function getMatchingValue(equation, regx) {
    if (regx === 'ERROR') return 0;
    const _equation = equation;
    const charOccurance = _equation.search(regx);
    let counter = 0;
    let coefficient = '';
    while (counter < charOccurance) {
        coefficient = coefficient + equation.charAt(counter);
        counter++;
    }
    return (coefficient === '-')
        ? '-1'
        : (coefficient === '+' || coefficient === '')
            ? '1'
            : coefficient;
}
/**
 * Return Roots of the equation
 * @param {any} a Coefficient
 * @param {any} b Coefficient
 * @param {any} c Coefficient
 * @returns Array containing roots
 */
function getRoots(coeffList) {
    const [a, b, c] = coeffList;
    if (a === 0) {
        if (b === 0) {
            if (c === 0) {
                return [];
            } else {
                return [];
            }
        } else {
            // Linear Equation
            return [-c / b];
        }
    }
    let d = b * b - 4 * a * c;
    if (d < 0) {
        return [];
    }
    const b1 = eval(- + b);
    if (d === 0) {
        const r = -0.5 * b1 / a;
        return [r, r];
    }
    d = Math.sqrt(d);
    return [(b1 - d) / (2 * a), (b1 + d) / (2 * a)];
}

console.log(solveQuadraticEquation('4/2x^2 - 20/2x + 24/2'));
console.log(solveQuadraticEquation('2x^2 - 10x + 12'));
console.log(solveQuadraticEquation('2x^2 + 10x'));
console.log(solveQuadraticEquation('2x^2 - 12'));
console.log(solveQuadraticEquation('10x + 12'));
console.log(solveQuadraticEquation('0 * x^2 - 10 * x + 12'));
console.log(solveQuadraticEquation('12'));
\$\endgroup\$
4
\$\begingroup\$

What an improvement!

... and now it wont fall over at the slightest misplaced character.

There is one bad point.

function getRegexSource(regex) {
    const _regex = regex;

function getCoefficients(equation) {
    // Used eval instead of parseFloat to support Fractions
    const _equation = equation;

Dont copy the arguments to another variable for no reason. You copy variables to

  1. create a less noisy alias, if the variable starts to pad out code lines

    function getCoefficients(equation) {
        const equ = equation;  // as an alias
    
  2. If you need to preserve the variable

     function foo(someVal) { 
         const rollBack = someVal;
    
         ... code in a try
         someVal += "some changes";
    
         catch(e){
              return rollBack;
         }
         return someVal
    

Doing it for no reason just adds noise which is never good. Also creating copies can be dangerous, particularly the underscored variety.

_underscore is not a very visible marker. We read by shape and the underscore can be missed. See your getMatchingValue function where you have used the wrong variable equation rather than the _equation . Though it does not mater in this case, it could have been a bug if the variable with the underscore was being changed, and a hard to spot one at that.

Some style points.

  • Don't use block-less statement blocks.

    // this is not good style    
    if(foo === bar) bar ++;
    
    // and this is even worse
    if(foo === bar) 
         bar ++;
    

    Always add the block delimiters {} the time saved typing is not worth the time that can be lost via this short cut (its insidious evil). As any long term C like language coder will attest to, when you modify the code you can miss the { } and create a very very hard to spot bug.

    // example in some code noise to illistrate the problem
    //  you may add another line as so statement block.
    bar = someInput();
    foo = getInfo();
    if(foo === bar) 
         bar ++;
         foo --;
    show(foo);
    show(bar);
    // sittine inside a lot of code and with the indent the missing {}
    // can easily be overlooked.
    

    To prevent this ever happening always delimit blocks

     if(foo === bar) { bar++ }
    
     // or
     if(foo === bar) { 
          bar++;
     }
    
  • Encapsulate the core functionality.

    You can wrap the whole thing in a object/class/export exposing only what you need to solve an equation. This will make the functionality portable and reusable.

    As it is, it exposes a lot of named functions to the global scope that may clash with other code if you are creating larger projects.

  • Use the "use strict" directive

    For extra safety add the "use strict" directive to the top of every JS file as an extra protection against common bad coding habits (not that I spotted any). Note that if you use modules the use strict mode is automatic.

Summing up.

This is a vast improvement over the previous version. There are some logic inefficiencies in how you handle the ternary tests to find the RegExp for each coefficient. ( RegExp have built in logic operations, you seldom need to do it in code)

If you find yourself nesting more than 2 ternary expressions there is likely a better simpler and easier to read solution.

Getting the roots is but a minor part of the code. You will get a lot more miles out of it if you think of the whole thing as a polynomial equation parser that returns the polynomial equation object. To store any polynomial all you need is the coefficients. Then it would be

const equ = polynomial.parse("x^2+3x-4"); // returns an equation object
console.log(equ.roots());

Roots is just one of the functional properties of the equation, some others like toString(), toFunction(), y(x), derivative(), antiDerivative(), minima() maxima(), and all the other useful stuff that a set of Coefficients can represent.

So all up, great work.. :)

\$\endgroup\$
  • \$\begingroup\$ What you write at the last has inspired me to write another programme. :) \$\endgroup\$ – Shubhendu Vaid Apr 20 '18 at 14:53

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.