3
\$\begingroup\$

I just spent the whole night writing this little script for my search engine. Now i want to improve it.

Any ideas?

<?php 
echo "KEYWORDS : ".  $searchstring  = "the and";

$sentence = "Skype Home is unavailable at the moment. Check back later to see your news and alerts.  It's easy to start a conversation on Skype:  Choose a contact and start talking Call a phone or mobile from the dial pad, or send an SMS.";

echo "<br><br><b>ORIGINAL STRING</b><br><br>$sentence";


$kt=split(' ',$searchstring); //Breaking the string to array of words

$num=1;

while(list($key,$val)=each($kt)){
    // do not load sentence from session on first run!  

    if($num<=1){
    } else {
        $sentence  = $_SESSION['NewSentence'];
    }
    if($val<>" " and strlen($val) > 0){

        // make keyword bold and send entire sentence to session

        $_SESSION['NewSentence']    = eregi_replace ("$val", "<b>$val</b>", $sentence);
        // output sentence with added keyword

        echo "<br><br>$num bolding:  <b style='color:green;'>$val</b> <br><br>  ".$_SESSION['NewSentence'];

        $num++;
    }
}

// make all bold keywords to red`

echo '<br><br><b>FINAL STRING</b><br><br>';
echo  $_SESSION['NewSentence'] = eregi_replace ("<b>", "<b style='color:red'>", $_SESSION['NewSentence']);

//destroy  session
unset($_SESSION['NewSentence']);
session_unregister($_SESSION['NewSentence']);?>
\$\endgroup\$

2 Answers 2

Reset to default
5
\$\begingroup\$

Use of deprecated functions

Both split() and eregi_replace() are deprecated since PHP 5.3. Don't use them.

Use preg_replace() instead of eregi_replace() and explode() instead of split() Other alternatives are str_replace() and preg_split().

Unexpected results

When the search string is lowercase, all replaced words will be lowercase too. The case of the original word is not preserved.

Unused variables

The $key variable is assigned but never used. In fact, the entire while loop is odd. Use a foreach loop instead, like so:

foreach($kt as $val) {
    ...

Missing function

The code should be in a function. The function would require two parameters, the string and an array of words to highlight, and return the result string. The function should not echo anything (but I presume the echo statements are debugging things).

Old HTML

Although valid, the use of <b> tags is outdated. It describes formatting in a document layout, without specifying the need for the format. You should use html for layout and css for styling. Then you can replace the <b> with a <span class="highlight> (or any other sensible class name).

\$\endgroup\$
3
\$\begingroup\$

Unused if statements

You have a if($num<=1) statment that is empty, while your using else. Reverse the statement like this.

if ($num > 1) {
    $sentence  = $_SESSION['NewSentence'];
}

Ambiguous use of $_SESSION

You should not use $_SESSION as a storage unless you are going to use it in another session (read: new page load/refresh). At the end of your script you're destroying the session, if that's your intention use variables as placeholders and leave $_SESSION alone.

Hard to read code

You should always aim to make the code as readable as possible. Take for example look at PSR-1 and PSR-2 to learn a coding standard.

\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.