Improve my code : Distributing the content of a textarea in two different arrays depending on markers.

Question

Hi,

I have a textarea where the user can create a feature list with a title for each block of features. The idea is to store the [title] and the features in two different MySQL tables.

[Outdoor]
BBQ
Tennis court
Swimming pool

[Internal Equipment]
DVD Player
Plasma screen

Here is what I've done so far; it works but it's not neat:

<form name="form" method="get" action="">
  <p>
    <textarea name="content" cols="35" rows="12" id="content"><? 
if (isset($_GET['content'])) echo $_GET['content']; ?></textarea>
  </p>
  <p>
    <input name="parse" type="submit" id="parse" value="Parse">
  </p>
</form>
<?php

if(isset($_GET['parse']))
{
   $content = $_GET['content'];
   $content = preg_replace("/(^[\r\n]*|[\r\n]+)[\s\t]*[\r\n]+/", "\n", $content);
   $content = trim($content);

   $content1 = preg_replace('/\r\n|\r/', "\n", $content );  
    $data = explode("\n", $content1); 


    $p=0;
   foreach ($data as $title) {
   if (substr_count($title, '[')||substr_count($title, ']')){
  $p++;
   $arr[$p]=$title;

   }else {
   $g[$p][]=$title;
   }
   }

    print_r($arr); 
    echo '<br />';
    print_r($g);
}
?>

Thanks for your ideas.

Answer 1

Other than that, make sure to use the POST method instead in your form. The query string vars can easily be tempered with.

Answer 2

The code looks fine, for the most part.

The issue I see is that you're not sanitizing user input, rather you're displaying it directly:

if (isset($_GET['content'])) echo $_GET['content']; 

At the very least, use strip_tags():

if (isset($_GET['content'])) echo strip_tags($_GET['content']));

Also, you should probably use POST instead of GET.

Edit:

Another thing that I noticed is inconsistent use of braces. Either use the K&R style:

if (some_condition) {
    code
}

Or put them on a separate line (my preferred approach):

if (some_condition)
{
    code
}

(Does anyone know if there is a name for this style?)

Same thing for indentation. Keep it consistent. This is just a style issue, but it impacts the legibility of your code.

Answer 3

Is this neat enough for you?

$result = array();
$content = array_filter(array_map('trim', explode('[', $_GET['content'])), 'strlen');

foreach ($content as $value)
{
    $value = array_map('trim', explode("\n", $value));
    $result[rtrim(array_shift($value), ']')] = $value;
}

And the output:

echo '<pre>';
print_r($result);
echo '</pre>';

Array
(
    [Outdoor] => Array
        (
            [0] => BBQ
            [1] => Tennis court
            [2] => Swimming pool
        )

    [Internal Equipment] => Array
        (
            [0] => DVD Player
            [1] => Plasma screen
        )

)

I suppose you know what to do with the $result array? Something like:

foreach ($result as $title => $features)
{
    // INSERT INTO foo (title) VALUES ($title);

    foreach ($features as $feature)
    {
        // or INSERT INTO bar (title, features) VALUES ($title, $feature);
    }
}