Suggestions for Shorter Approach

JavaScript
#1

Tell us what’s happening:
Describe your issue in detail here.
My code is running perfectly.But can anyone suggest how can I shorter this code as its too lengthy approach.

  **Your code so far**


function smallestCommons(arr) {
var small=Math.min(...arr);
var big=Math.max(...arr);
var array=[];
var i=0;
var j=0;
//////Create the array of number range
for(i=small;i<=big;++i){
   array.push(i);
}
////Create array of prime numbers less than bigger number
var array_prime=[2];
for(i=3;i<=big;++i){
  var value=i;
  //console.log(value);
  var flag=-1;
  for(j=2;j<value;++j){
    if(value%j==0){
      flag=1;
      break;
      }
    }
    if(flag==-1){
      array_prime.push(value);
      }
  }
  console.log(array_prime);
  
  //Divide whole array by each element of prime number array
  var arr_new=[];
  for(i=0;i<array_prime.length;++i){
    var count=0;
    var num=array_prime[i];
    for(j=0;j<array.length;++j){
         if(array[j]>=num){
           if(array[j]%num==0){
             count=1;
             array[j]=array[j]/num;
           }
         }
     //console.log(array);   
    }
    if(count==1){
    arr_new.push(num)};
  }
console.log("Original array=",array);
  
  var new_array=[];
  for(i=0;i<array.length;++i){
    if(array[i]!==1){
      new_array.push(array[i]);
    }
  }
  new_array=new_array.sort();
  for(i=0;i<new_array.length;++i){
    var ans=new_array[i];
    if(array_prime.includes(ans)){
      arr_new.push(ans);
      for(j=0;j<new_array.length;++j){
        if(new_array[j]>=ans && new_array[j]%ans==0){
          new_array[j]=new_array[j]/ans;
        }
      }
    }
  }
  console.log(arr_new);
  console.log(new_array);
  var product=1;
  for(var i=0;i<arr_new.length;++i){
    product*=arr_new[i];
    console.log(product);
  }
  console.log(product);
  return product;
}



smallestCommons([23,18]);

  **Your browser information:**

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Safari/537.36

Challenge: Smallest Common Multiple

Link to the challenge:

#2

I delete all you comments so any comment you see I wrote:

function smallestCommons(arr) {
var small=Math.min(...arr);
var big=Math.max(...arr);
//you can simply do this:
//var array = [Math.min(...arr), Math.max(...arr)]
var array=[];

//You don't need to ever make i or j outside
//the context of a for loop
//this would be different if you were a 
//while loop
var i=0;
var j=0;

for(i=small;i<=big;++i){
   array.push(i);
}

//This is an improper naming convention for JS
//it should be arrayPrime
var array_prime=[2];
for(i=3;i<=big;++i){
  //you don't need value you already have i
  var value=i;
  
  var flag=-1;
  for(j=2;j<value;++j){
    if(value%j==0){
      flag=1;
      break;
      }
    }
    //this if could be inside the above loop so you 
    //wouldn’t need flag
    if(flag==-1){
      array_prime.push(value);
      }
  }
  
  
  //Divide whole array by each element of prime number array
  var arr_new=[];
  for(i=0;i<array_prime.length;++i){
    var count=0;
    var num=array_prime[i];
    for(j=0;j<array.length;++j){

      //you don't need to ifs
      //instead use the since they both must be true
      //us the && operator between them
      //in the same statement 
         if(array[j]>=num){
           if(array[j]%num==0){
             count=1;
             array[j]=array[j]/num;
           }
         }   
    }
    if(count==1){
    arr_new.push(num)};
  }

  
  var new_array=[];
  for(i=0;i<array.length;++i){
    if(array[i]!==1){
      new_array.push(array[i]);
    }
  }
  //sort is a destructive method so
  //you can simply call the method
  new_array=new_array.sort();
  for(i=0;i<new_array.length;++i){
    var ans=new_array[i];
    if(array_prime.includes(ans)){
      arr_new.push(ans);
      for(j=0;j<new_array.length;++j){
        if(new_array[j]>=ans && new_array[j]%ans==0){
          new_array[j]=new_array[j]/ans;
        }
      }
    }
  }
 
  var product=1;
  for(var i=0;i<arr_new.length;++i){
    product*=arr_new[i];
    console.log(product);
  }
  
  return product;
}



smallestCommons([23,18]);

I did not read the entirety of you code, but the main two things I see making it longer are:

You make a lot of variables that don’t need to exist.

That vast majority of extra code is purely due to implementation, and I would recommend that you research effective ways of finding primes.

1 Like
#3

Hi @riti0301 !

I added spoiler tags around your code since it is a working solution and we don’t want to spoil it for those who have not worked on this challenge.

1 Like
#4 
function smallestCommons(arr) {

  //populate the array of numbers
  let min=Math.min(...arr)
  let max=Math.max(...arr)
  let fullArr=[]
  for (let i=min; i<=max; i++){
    fullArr.push(i)
  }

  //test if a number is common multiple for all items
  function compare(num){
    return fullArr.every(n=>num%n==0)
  }
  
  // set from where to start comparing numbers...
  // (largest number * second largest)
  let multiple=max*(max-1)

  while(!compare(multiple)){
    // go thru multiples of the largest number in the list, ...
    // until it finds one that is valid for all other numbers in the list
    multiple+=max 
  }

  // return the smallest number which is  evenly divisible by all
  return multiple;
}

Hello this is a solution i made and added some comments so you can follow the code. It should be pretty efficient, altho one could improve it even more

1 Like
#5

If you want a much shorter solution, try using Euclidean algorithm.

1 Like