3
\$\begingroup\$

I wrote a search script, now I want a more efficient way of finding the keywords used for the search and make them bold. I used this code but it I feel it could be greatly improved on. Thanks for any suggestion.

// I use this code to select the lines to display
$lines = explode('.', trim($search_result));

// Break up the result into sentences
$j = 0; $line = '';
for($i = 0; $i < sizeof($lines); $i++) {
    // Look for the keywords in each of the line        
    if (strstr($lines[$i], $keywords[0]) || strstr($lines[$i], $keywords[1]) || 
        strstr($lines[$i], $keywords[2]) || strstr($lines[$i], $keywords[3])
    ) {
        $j++;
        $line .= trim($lines[$i])."...";
    }

    if ($j == 2)
        return $line;

    // If no keyword is found in first 100 lines/sentences, just display the first 3 lines
    if ($i == 100)
        return $lines[0]."...".$lines[1]."...".$lines[2];

}

I use this code to make the keywords bold

function findAndReplace($keywords, $sentence) {
    for ($i=0; $i<sizeof($keywords); $i++)
        $words[] = "<b>".ucfirst($keywords[$i])."</b>";

    return str_ireplace($keywords, $words, $sentence);
}
\$\endgroup\$
3
  • \$\begingroup\$ Please pardon how bad the code looks. I'm using a mobile device for this \$\endgroup\$
    – Chibuzo
    Commented Sep 10, 2011 at 8:25
  • \$\begingroup\$ Next time format your code, keep the indentation consistent if you want others to help. Now, where exactly do you feel it can be greatly improved on? What is not working the way you expect it to? \$\endgroup\$
    – Shef
    Commented Sep 10, 2011 at 8:25
  • \$\begingroup\$ The script is kind of inconsistent - it searches articles using the entered keywords, if it retrieves results, it may display topic, part of the body (of which I included the code for) and the article url, for some articles and only display the topic and url alone for the unlucky articles. \$\endgroup\$
    – Chibuzo
    Commented Sep 10, 2011 at 8:55

2 Answers 2

2
\$\begingroup\$

A couple of small things...

I would replace this:

if (strstr($lines[$i], $keywords[0]) || strstr($lines[$i], $keywords[1]) || 
    strstr($lines[$i], $keywords[2]) || strstr($lines[$i], $keywords[3])

with something like:

foreach($keyword in $keywords) {
    if (strstr($lines[$i]) {
       $j++;
       $line .= trim($lines[$i])."...";
       break;
    }
 }

Also, this line:

       $line .= trim($lines[$i])."...";

will result in an extra trailing "..." when $j gets to 2. I'm presuming you want something like "line1...lineN", but instead you'll get "line1...lineN...". You probably want to chop off those extra dots, or only add them when needed.

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

Just one note:

for($i = 0; $i < sizeof($lines); $i++) { ... }

Calling sizeof() in every iteration could be slow. I'd write the following:

$size = count($lines);
for($i = 0; $i < $size; $i++) { ... }

Note that sizeof was changed to count as @Yannis Rizos suggested in the comments.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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