How to simplify javascript program?

Question

I would love to minimize the program. Maybe putting p1-16 in one line of code, same with count and gefunden? Since my language skills are minimal I can't find the right information.

It would also be great if there was a way to minimize the if else statements in search hits pdf.

Right now I do the code by hand to add new pdfs, as in search hits pdf1 to pdf2. Any easier way would greatly help me.

function Suche(str){
    p1=document.getElementById('pdf1').innerHTML;
    p2=document.getElementById('pdf2').innerHTML;
    p3=document.getElementById('pdf3').innerHTML;
    p4=document.getElementById('pdf4').innerHTML;
    p5=document.getElementById('pdf5').innerHTML;
    p6=document.getElementById('pdf6').innerHTML;
    p7=document.getElementById('pdf7').innerHTML;
    p8=document.getElementById('pdf8').innerHTML;
    p9=document.getElementById('pdf9').innerHTML;
    p10=document.getElementById('pdf10').innerHTML;
    p11=document.getElementById('pdf11').innerHTML;
    p12=document.getElementById('pdf12').innerHTML;
    p13=document.getElementById('pdf13').innerHTML;
    p14=document.getElementById('pdf14').innerHTML;
    p15=document.getElementById('pdf15').innerHTML;
    p16=document.getElementById('pdf16').innerHTML;
    p17=document.getElementById('pdf17').innerHTML;
    gefunden1=0;
    gefunden2=0;
    gefunden3=0;
    gefunden4=0;
    gefunden5=0;
    gefunden6=0;
    gefunden7=0;
    gefunden8=0;
    gefunden9=0;
    gefunden10=0;
    gefunden11=0;
    gefunden12=0;
    gefunden13=0;
    gefunden14=0;
    gefunden15=0;
    gefunden16=0;
    gefunden17=0;
    count1=0;
    count2=0;
    count3=0;
    count4=0;
    count5=0;
    count6=0;
    count7=0;
    count8=0;
    count9=0;
    count10=0;
    count11=0;
    count12=0;
    count13=0;
    count14=0;
    count15=0;
    count16=0;
    count17=0;
    searchstring=str;
    
    
    //Search Hits PDF1
    
    endsearch=p1.length;
    weiter=1;
    
    
    if(p1.indexOf(str)>-1){
       gefunden1=1;
       pos1=p1.indexOf(str)+searchstring.length;
       count1=count1+1;}
    else{weiter=0;}
    
    for(i = 1; i <=10; i++){
       if(weiter==1){
          if(p1.indexOf(str,pos1)>-1){
             pos2=p1.indexOf(str,pos1)+searchstring.length;
             if (pos2<=endsearch){
                if(count1<10){
                   count1=count1+1;
                   pos1=pos2;}
                else{
                   count1="Mehr als 10";
                   pos1=pos2;}}
             else{
                weiter=0;}}
          else{
             weiter=0;}}}
    
    
    //Search Hits Pdf2
    
    
    endsearch=p2.length;
    weiter=1;
    
    if(p2.indexOf(str)>-1){
       gefunden2=1;
       pos1=p2.indexOf(str)+searchstring.length;
       count2=count2+1;}
    else{weiter=0;}
    
    
    for(i = 1; i <=10; i++){
       if(weiter==1){
          if(p2.indexOf(str,pos1)>-1){
             pos2=p2.indexOf(str,pos1)+searchstring.length;
             if (pos2<=endsearch){
                if(count1<10){
                   count2=count2+1;
                   pos1=pos2;}
                else{
                   count2="Mehr als 10";
                   pos1=pos2;}}
             else{
                weiter=0;}}
          else{
             weiter=0;}}}

and so on....

Answer 1

Why not use an array called p?

const p = []
for (let i=1; i<18; i++) {
    p.push(document.getElementById(`pdf${i}`).innerHTML)
}

You can do the same for gefunden and count. The rest of your code, if repetitive, could go in a function and be called in another for loop.

Answer 2

You can use arrays.

Arrays, in a simple way, put lots of info into one variable. For example:

var this_is_an_array=[0,1,2,3,4,5]

Arrays count from 0 and onwards, so do take note to start counting from 0

More in detail at w3 schools: https://www.w3schools.com/js/js_arrays.asp

Answer 3

I agree that this should be on code review. But you have a working code and ask how to make it better. So here you go.

• replace all those variables that have the format variableN with an array. As soon as you have such a naming format you most of the time either want to use an array or change the name.

• And you definitely want to clean up that function that searches for the occurrence of the given string.

• Always define variables using const or let. And add comments to the code.

• If something reflects a boolean, then use one instead of 0 or 1.

• Make use of comments, that will also help others when looking at your code (and it also helps you if you look at your code after a while).

• Use variables instead of magic numbers like 10.

• and even if your preferred language is not the one used of the programming language, you should stick with the one the programming language use.

So here is a reworked version of your code:

// use an options parameter to make the function more flexible
function search(str , options) {
  // destructuring the options into variables 
  const {maxCount, pdfCount} = options;

  const items = [];

  for (let i = 1; i <= pdfCount; i++) {
    items.push({
      p: document.getElementById(`pdf${i}`).innerHTML,
      found: false,
      count: 0
    })
  }

  items.forEach(item => {
    let count = 0;
    let currentPosition = 0; // position where to start searching
    let foundAtPosition;

    // do-while loop to do at least one search
    do {
      foundAtPosition = item.p.indexOf(str, currentPosition);
      
      // check if we found an occurence
      if (foundAtPosition != -1) {
        // increase the count
        count++;
        // set the current position after the found occurence
        currentPosition = foundAtPosition + str.length;
      }
      
      // if we found more then maxCount we can leave the loop
      if (count > maxCount) {
        break;
      }

      // only continue the loop when something was found
      // you could move "count > maxCount" to the while condition
      // but the main purpose of the while loop is to iterate over the content
    } while (foundAtPosition != -1);

    // do the composing the information to be set for the item after the for loop, 
    // that makes many things clearer as it is not part of the searching process

    // set found to true or false by checking if count is larger then 0
    item.found = count > 0;

    
    if (count > maxCount) {
      item.count = `Mehr als ${maxCount}`;
    } else {
      item.count = count;
    }
  })

  return items;
}

console.dir(search('hey', {maxCount: 10, pdfCount: 3}))

<div id="pdf1">
heyheyaaheyaaahey
</div>
<div id="pdf2">
heyheyaaheyaaahey
heyheyaaheyaaahey
heyheyaaheyaaahey

</div>
<div id="pdf3">
foo
</div>

You could also utelize str.split([separator[, limit]]) as mention here How to count string occurrence in string? and utilize the limit function.

But if you do that you really need to document the item.p.split(str, maxCount+2).length - 1 construct because that's hard to understand otherwise.

// use an options parameter to make the function more flexible
function search(str , options) {
  // destructuring the options into variables 
  const {maxCount, pdfCount} = options;

  const items = [];

  for (let i = 1; i <= pdfCount; i++) {
    items.push({
      p: document.getElementById(`pdf${i}`).innerHTML,
      found: false,
      count: 0
    })
  }

  items.forEach(item => {
    // use maxCount + 2 to figure out if we have more then max count subtract 1 from length to get the actual count
    const count = item.p.split(str, maxCount+2).length - 1

    // set found to true or false by checking if count is larger then 0
    item.found = count > 0;

    if (count > maxCount) {
      item.count = `Mehr als ${maxCount}`;
    } else {
      item.count = count;
    }
  })

  return items;
}

console.dir(search('hey', {maxCount: 10, pdfCount: 3}))

<div id="pdf1">
heyheyaaheyaaahey
</div>
<div id="pdf2">
heyheyaaheyaaahey
heyheyaaheyaaahey
heyheyaaheyaaahey

</div>
<div id="pdf3">
foo
</div>