-4
\$\begingroup\$

Am trying to write a function that takes an array as a parameter, and then returns the highest and lowest number from the array without sorting the array. Below is my code, How can I make it better.

function lowestAndHighest(arr) {
    this.lowest = 0;
    this.highest = 1;

    //This function returns the lowest number
    this.lowestNum = () => {
        arr.forEach(elem => {
            if(this.lowest > elem) {
                return this.lowest =  elem;    
            } 
        });
        return this.lowest;
    }

    //This function returns the higest number
    this.highestNum = () => {
        arr.forEach(elem => {
            if(this.highest < elem) {
                this.highest = elem;
            }
        });
        return this.highest;
    }
}

let num = new lowestAndHighest([30,3,6,8,1,0,-10, -60]);
console.log(num.lowestNum());
console.log(num.highestNum());
\$\endgroup\$

closed as off-topic by 200_success, esote, yuri, pacmaninbw, Graipher May 12 at 17:42

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – 200_success, esote, yuri, pacmaninbw, Graipher
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ Unless I am mistaken, your code would compute the lowest number in [3, 4, 5] as 0, and the highest number in [-3, -4, -5] as 1. \$\endgroup\$ – Martin R May 11 at 14:03
  • \$\begingroup\$ Yes exactly the problem \$\endgroup\$ – Iwowo Chisom May 11 at 14:06
  • 2
    \$\begingroup\$ Welcome to CR! I'm voting to close this as off-topic because the code is not working as intended. Having said that, why not simply use builtins: Math.max(...[1,4,5,6,7])? \$\endgroup\$ – ggorlen May 11 at 14:48
1
\$\begingroup\$

How can I make [the code presented] better

Give Test First a try.

Document your source code, in the source code: what is the goal of "any" given piece? ("The This function returns the higest number comments" are a step in the right direction. (Your IDE doesn't check spelling?))

Be consistent. (In addition to a double blank,) There is an irritating difference between

if(this.lowest > elem) {
    return this.lowest =  elem;
}

and

if(this.highest < elem) {
    this.highest = elem;
}

If your goal was to keep the number of comparisons low, there is a simple technique taking you half way to optimal.

(For all I don't know about ECMAScript & JSDoc, The Simplest Thing That Could Possibly Work might be

/* Returns minimum and maximum value in values
 * @param {Array<number>} values */
function lowestAndHighest(values) {
    return [ Math.min.apply(Math, values),
             Math.max.apply(Math, values) ];
}

)

\$\endgroup\$
-1
\$\begingroup\$

I just saw my error now

function lowestAndHighest(arr) {

    this.lowest = arr[0];//corrected
    this.highest = arr[0];//corrected

    //This function returns the lowest number
    this.lowestNum = () => {
        arr.forEach(elem => {
            if(this.lowest > elem) {
                return this.lowest =  elem;    
            } 
        });
        return this.lowest;
    }

    //This function returns the higest number
    this.highestNum = () => {
        arr.forEach(elem => {
            if(this.highest < elem) {
                this.highest = elem;
            }
        });
        return this.highest;
    }
}

let num = new lowestAndHighest([30,3,6,8,1,0,-10, -60]);
console.log(num.lowestNum());
console.log(num.highestNum());
\$\endgroup\$
  • \$\begingroup\$ Now, what is the result for Martin R's arrays? \$\endgroup\$ – greybeard May 12 at 3:30
  • \$\begingroup\$ It is working as intended \$\endgroup\$ – Iwowo Chisom May 12 at 15:18
  • \$\begingroup\$ I misread the lambda assigned to this.lowestNum - irritating for the extraneous return the lambda assigned to this.highestNum doesn't show. \$\endgroup\$ – greybeard May 12 at 17:30

Not the answer you're looking for? Browse other questions tagged or ask your own question.