3
\$\begingroup\$

My code takes the input submitted by $ _POST, via a checkbox array. I process the array and check the sent data if there is a particular string in it, which is set as my default. If I have a string, which is my default, I just go through implode and add ",", if my default string is missing in the data. I add and sort the array anew, with the first string being the default. I want to shorten the code because I can't make it shorter.

<?php
if(isset($_POST['submit'])) {
    //for update
    $site_id['locale'] = 'tr';
    
    //new
    $exampleArray = isset($_POST['lala']) ? $_POST['lala'] : '';
    
    $example = null;
    if(($key = array_search($site_id['locale'], $exampleArray, true)) !== false) {
        unset($exampleArray[$key]);
        
        $FirstString = array($site_id['locale']);
        $exampleArray = array_diff($exampleArray, $FirstString);
        usort($exampleArray);
        $exampleArray = array_merge($FirstString, $exampleArray); 
        //print_r($exampleArray);
        $myArray = array();
        foreach ( $exampleArray as $key => $value ) {
            $myArray[] = $value;
        }
        echo implode( ', ', $myArray );
    } else {
        $FirstString = array($site_id['locale']);
        $exampleArray = array_diff($exampleArray, $FirstString);
        usort($exampleArray);
        $exampleArray = array_merge($FirstString, $exampleArray); 
        //print_r($exampleArray);
        $myArray = array();
        foreach ( $exampleArray as $key => $value ) {
            $myArray[] = $value;
        }
        echo implode( ', ', $myArray );
    }
    
}
?>
<form method="POST" action="">
<input type="checkbox" name="lala[]" value="bg" />
<input type="checkbox" name="lala[]" value="us" />
<input type="checkbox" name="lala[]" value="gr" />
<input type="submit" name="submit" />
</form>
\$\endgroup\$
1
  • \$\begingroup\$ Warning: usort() expects exactly 2 parameters, 1 given \$\endgroup\$ Commented Dec 19, 2020 at 23:18

2 Answers 2

Reset to default
4
\$\begingroup\$

Addressing two issues here:

Setting a default value

PHP now has a null coalesce operator (??). This allows you to easily set a default value if the variable is null.

$exampleArray = $_POST['lala'] ?? [];

(Here, I am setting the default value to an empty array)

Refactoring code

Any time you catch yourself copy/pasting a chunk of code, that’s a signal to start refactoring. So the code you provided had almost everything in the if and else block repeated. Just pull the repeated stuff out. In this case, you don’t even need the else.


if(isset($_POST['submit'])) {
    //for update
    $site_id['locale'] = 'tr';
    
    //new
    $exampleArray = $_POST['lala']) ?? [];
    
    $example = null;
    if(($key = array_search($site_id['locale'], $exampleArray, true)) !== false) {
        unset($exampleArray[$key]);
    }
        
    $FirstString = array($site_id['locale']);
    $exampleArray = array_diff($exampleArray, $FirstString);
    usort($exampleArray);
    $exampleArray = array_merge($FirstString, $exampleArray); 
    //print_r($exampleArray);
    $myArray = array();
    foreach ( $exampleArray as $key => $value ) {
        $myArray[] = $value;
    }
    echo implode( ', ', $myArray );

    // Note, should always redirect after a post. 
    // Post means change data, so after you have 
    // stored the changes, redirect to the next (or 
    // back to same) page. Cannot be any output 
    // before this (print, echo, blank lines, html)
    
}
```
\$\endgroup\$
0
3
\$\begingroup\$

To be honest, I am struggling to follow the intended logic in your snippet.

To my understanding, you are merely wanting to check if the default value exists in the submitted array -- if not, add the default value as the first element. This can be far simpler, more direct.

Code: (Demo)

$site_id['locale'] = 'tr';
$exampleArray = ['us', 'gr'];

if (!in_array($site_id['locale'], $exampleArray)) {
    array_unshift($exampleArray, $site_id['locale']);
}

echo implode(', ', $exampleArray);

Output:

tr, us, gr

As for the code in general, I endorse Tim's insights regarding the null coalescing operator and using an array versus an empty string when null. A string will make array_search() choke.

I do not know your intended sorting logic with usort(). This is a broken part of your code and must be changed or removed.

If your code intends to validate/sanitize the submitted values, I can't find that logic. It does make sense to compare the submitted values against a lookup array.

Also, about $_POST, I don't know what actions are occurring in this script execution, but if you are only reading data from the server (versus writing to the server), then you should be using $_GET. $_POST is primarily used when you are changing the file system or the database (there are some exceptions to this general rule).

\$\endgroup\$
5
  • \$\begingroup\$ Thanks. @mickmackusa That seems to be exactly what I was looking for. Thanks. Question: Before processing an array with in_array, isn't it more correct to check with is_array whether the data is an array? Just a joker, if you can? \$\endgroup\$ Commented Dec 19, 2020 at 23:35
  • 1
    \$\begingroup\$ The submitted element will be an array unless the user manually fools with the form or submission data structure. To ensure that you are always using an array in this case, you can cast the variable as array-type. $selectedLanguages = (array)($_POST['lala'] ?? []); then any scalar values will become the lone element of the newly formed array. \$\endgroup\$ Commented Dec 20, 2020 at 0:37
  • 1
    \$\begingroup\$ @Vladimirov it makes perfect sense to check that it is an array in the first place. Just casting to array means that you are generous but ignorant. You graciously guess the intent of the one who tampered your front-end form and desperately try to satisfy him. You better disregard such tamperer. Better yet log that event as it means someone is playing tricks with your back-end. \$\endgroup\$ Commented Dec 21, 2020 at 4:40
  • 1
    \$\begingroup\$ Rule #7: "Don't let people play with your back-end -- unless you give them express permission to do so" ;) \$\endgroup\$ Commented Dec 21, 2020 at 4:42
  • \$\begingroup\$ I'm frankly surprised that my answer wasn't accepted (not that it matters) because I demonstrated how 27 lines of convoluted code can be simplified to form 6 lines of readable code. \$\endgroup\$ Commented Dec 28, 2020 at 22:25

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.