Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The code it self is pretty self explanatory, however i do not think it is very efficient at all. Is there a better way to do this?

Its basically so i can track spam sites and save the sites into my honeypot db.

<?php
$site = $_GET['url'];
$con=mysqli_connect("localhost","_","","_");
// Check connection
if (mysqli_connect_errno()) {
  echo "Failed to connect to MySQL: " . mysqli_connect_error();
}

//Gambling
if (strpos($site, "betting") ||strpos($site, "blackjack") ||strpos($site, "casino") ||strpos($site, "craps") ||strpos($site, "football") ||strpos($site, "gamble") ||strpos($site, "gambling") ||strpos($site, "game") ||strpos($site, "gaming") ||strpos($site, "greyhound") ||strpos($site, "highroller") ||strpos($site, "lottery") ||strpos($site, "lotto") ||strpos($site, "odds") ||strpos($site, "poker") ||strpos($site, "racehorse") ||strpos($site, "racing") ||strpos($site, "roulette") ||strpos($site, "royalflush") ||strpos($site, "slotmachine") ||strpos($site, "slots") ||strpos($site, "slotsmachine") ||strpos($site, "sport") ||strpos($site, "wager") ||strpos($site, "paigow") ||strpos($site, "bingo") ||strpos($site, "baccarat"))
{
    //Insert into gambling
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`gambling` (`id`, `url`) VALUES (NULL, '$site');");

}
//Payday
if (strpos($site, "payday") ||strpos($site, "loan") ||strpos($site, "bank") ||strpos($site, "money") ||strpos($site, "wonga") ||strpos($site, "lender") ||strpos($site, "credit") ||strpos($site, "debt") ||strpos($site, "repayment") ||strpos($site, "mortgage"))
{
    //Insert into payday
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`payday` (`id`, `url`) VALUES (NULL, '$site');");
}
//Lawyer
if (strpos($site, "law") ||strpos($site, "solicitor") ||strpos($site, "government") ||strpos($site, "injury") ||strpos($site, "personal") ||strpos($site, "accident") ||strpos($site, "advice") ||strpos($site, "traffic"))
{
    //Insert into lawyer
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`lawyer` (`id`, `url`) VALUES (NULL, '$site');");    
}
//Weight Loss
if (strpos($site, "weight") ||strpos($site, "loss") ||strpos($site, "pills") ||strpos($site, "diet") ||strpos($site, "garcinia") ||strpos($site, "cambogia") ||strpos($site, "acai") ||strpos($site, "berry") ||strpos($site, "raspberry") ||strpos($site, "ketone") ||strpos($site, "coffee") ||strpos($site, "tea") ||strpos($site, "health") ||strpos($site, "lose") ||strpos($site, "surgery") ||strpos($site, "fat") ||strpos($site, "dieting") ||strpos($site, "exercise") ||strpos($site, "workout") ||strpos($site, "gym") ||strpos($site, "hypnosis"))
{
    //Insert into weight loss
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`weightloss` (`id`, `url`) VALUES (NULL, '$site');");    
}
//Insurance
if (strpos($site, "ppi") ||strpos($site, "insurance") ||strpos($site, "payment") ||strpos($site, "claim") ||strpos($site, "calculator") ||strpos($site, "finance") ||strpos($site, "mis-sold"))
{
    //Insert into insurance
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`insurance` (`id`, `url`) VALUES (NULL, '$site');");     
}



mysqli_close($con);
exit();
?> 
share|improve this question
1  
Just can't wait for bobby tables to come along here –  Pinoniq Aug 4 at 22:00
    
If your connection fails, you echo some text, and then proceed to query the DB... is it me or does that not make sense at all. Quite similar to how "blackberry.com" would be blacklisted as a weight-loss site because strpos($site, 'berry') would test positive, but "lawless.com" would not (strpos($site, 'law'); would return a falsy 0) –  Elias Van Ootegem Aug 5 at 13:13

3 Answers 3

Why did you create multiple tables for each category?

Surely it would be better to have 1 single table with something like:

  • id - int
  • category - enum(lawyer, insurance, etc.)
  • url - varchar

You might want to have a look at this answer from StackOverflow for a better strpos alternative to your requirements: http://stackoverflow.com/a/9220624/367708

share|improve this answer
1  
What you call "url" should be called "slug" (because it not URL by definition) –  bad_boy Aug 4 at 15:27
    
I called it the same name as the original post to not create confusion. –  aLfa Aug 4 at 16:23

You have a large amount of duplication in your conditions. additionally you have helluva lot of hardcoded strings all over the place. I suggest you change your structure to be something like:

<?php
$gamblingStrings = ["betting", "blackjack", "casino", "football", /*... */];
//Same for your other categories and then

if (matchesCategory($site, $gamblingStrings)) 
{
    //insert into gambling
}

//and the corresponding function
private function matchesCategory($site, $strings) {
    foreach ($strings as $matcher) 
    {
         if(strpos($site, $matcher) !== false) 
         {
             return true;
         }
    }
    return false;
}

Disclaimer: My PHP is extremely rusty, the provided samples could be quite borked. But I hope you get the gist of it ;)

share|improve this answer

I would create an array of the search values then implode them when you want to evaluate using preg_match - something like this should work:

$rocket_newsites = array('betting','blackjack','sports'); //etc.

if(preg_match('/'.implode('|', $rocket_newsites).'/', $url, $matches))
{
      $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`gambling` (`id`, `url`) VALUES (NULL, '$site');");
}

This, to me, is far more readable and maintainable than having such long conditions, you just have to add or remove words from the array rather than touch the condition itself, and I would imagine that it offers better performance than all of those strpos, although I'm not too sure on that so don't take my word for it!

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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