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....

I feel like this would better fit on codereview.stackexchange.com as you don't have an actual problem — empiric
– empiric, Commented Sep 8, 2021 at 7:08
Using an array and store objects in it having the properties count, gefunden and p. — t.niese
– t.niese, Commented Sep 8, 2021 at 7:10
You only described why and how you don't like your current solution. To give you alternate perspectives, why not explain what it actually does, or should do? — Yoshi
– Yoshi, Commented Sep 8, 2021 at 7:10
How is that question different to the one you asked before For loop for repeatable action javasript?? — t.niese
– t.niese, Commented Sep 8, 2021 at 7:41

Arushi Gupta · Accepted Answer · 2021-09-08 07:31:42Z

4

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.

edited Sep 8, 2021 at 7:31

answered Sep 8, 2021 at 7:09

Arushi Gupta

1013 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

t.niese · Accepted Answer · 2021-09-08 08:35:30Z

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>

Dharman · Accepted Answer · 2021-09-08 07:15:19Z

0

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

edited Sep 8, 2021 at 7:15

Dharman♦

33.9k27 gold badges106 silver badges157 bronze badges

answered Sep 8, 2021 at 7:10

Ski3r3n

1291 silver badge10 bronze badges

1 Comment

t.niese Over a year ago

While w3schools does not suffer from the same serious problems it used to have, they are still full of inaccuracies. This is also true for your link. At one point they state

Arrays are Objects … Arrays are a special type of objects. The typeof operator in JavaScript returns "object" for arrays.

and later they say If you use named indexes, JavaScript will redefine the array to an object.. Arrays are objects, and setting a property (not named index) on them does not change anything. They are just not associative arrays where named indexes influence the length/size of the array.

Collectives™ on Stack Overflow

How to simplify javascript program?

3 Answers 3

Comments

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related