Logical clean up

Question

I am currently having logical structure errors and I can't seem to think of a proper way of writing it out so it looks clean. Currently, I have a table in my database named USER. User has 6 fields out of the many, named: first, second, third, fourth, fifth, sixth. What my code does is switch the data with two of the fields. So for example: the data in first is changed with the data in sixth.

Before: First - 1, Sixth - 6 After: First - 6, Sixth - 1

Currently, what I have is something that looks very messy, and I'm not sure how I can clean it up.

if($switch == true){ /* $switch just indicates if the numbers being switched is one of the fields i.e. first, second, etc.. */
                if($slot == 1){ // The number is being switched into the first slot
                    if($first == $number){
                        return 1; // error, can't switch with itself
                    } else if ($second == $number){
                        $temp = $first;
                        // 1st QUERY TO UPDATE THE DATA IN FIRST, WITH THE DATA IN THE SECOND
                        // 2nd QUERY TO UPDATE THE DATA IN SECOND, WITH THE INFO STORED IN VARIABLE $temp
                    } else if ($third == $number){
                        $temp = $first;
                        // 1st QUERY TO UPDATE THE DATA IN FIRST, WITH THE DATA IN THE THIRD
                        // 2nd QUERY TO UPDATE THE DATA IN THIRD, WITH THE INFO STORED IN VARIABLE $temp 
                    } // ... continues to check for 4, 5, 6...
                } else if($slot == 2){ /* Then checks to see if it was slot 2, i.e. Second */
                    // ....
                } else if($slot == 3){ /* Then checks to see if it was slot 3, i.e. Third */
                    // ....
                } else if($slot == 4){ /* Then checks to see if it was slot 4, i.e. Fourth */
                    // ....
                } else if($slot == 5){ /* Then checks to see if it was slot 5, i.e. Fifth */
                    // ....
                } else if($slot == 6){ /* Then checks to see if it was slot 6, i.e. Sixth */
                    // ....
                }

}

It continues for each slot, , 6 times each... I know it's bad programming but I have just started learning PHP (it's my first language, so bear with me). Any suggestions on what I can do to make this cleaner? Or even a better way of writing it? If anyone cares to explain to me over Skype or anything, I would be really grateful. Thank you.

Answer 1

You could do something like this:

// This function returns the column name for the specified slot
function getSlotColumn($slotNum)
{
    switch($slotNum)
    {
        case 1:
            retrun "first";

        case 2:
            return "second";

        ......
    }
}

then in the switching function

// This function swaps the values of the two specified slots
function swapValues($slot1Num, $slot2Num)
{
    if($slot1Num == $slot2Num) return;
    if($slot1Num < 1 || $slot1Num > 6) return;
    if($slot2Num < 1 || $slot2Num > 6) return;

    $slot1Col = getSlotColumn($slot1Num);
    $slot2Col = getSlotColumn($slot2Num);

            //get value of the first slot
    $slot1Val = //SELECT $slot1Col FROM ... WHERE ... slot

    // Now since we have the value of the first slot, we can update it from the second slot with this pseudo query
    //UPDATE ... SET $slot1Col = $slot2Col WHERE ...

    // Then update second slot from the value in $slot1Val
    //UPDATE ... SET $slot2Col = $slot1Val WHERE ...
}

Then use it like this:

if(switch == true)
{
    swapValues(1, 6);
}

Answer 2

Use a switch statement:

if($switch == true){ 
    switch($slot) {
        case 1 :
            if($first == $number){
                return 1; // error, can't switch with itself
            } else if ($second == $number){
                $temp = $first;
                // 1st QUERY TO UPDATE THE DATA IN FIRST, WITH THE DATA IN THE SECOND
                // 2nd QUERY TO UPDATE THE DATA IN SECOND, WITH THE INFO STORED IN VARIABLE $temp
            } else if ($third == $number){
                $temp = $first;
                // 1st QUERY TO UPDATE THE DATA IN FIRST, WITH THE DATA IN THE THIRD
                // 2nd QUERY TO UPDATE THE DATA IN THIRD, WITH THE INFO STORED IN VARIABLE $temp 
            } 
            break;
        case 2 :
            // ....
            break;
        case 3 :
            // ....
            break;
        case 4 :
            // ....
            break;
        case 5 :
            // ....
            break;
        case 6 :
            // ....
            break;
        default:
            // ...
    }    
}    

You will notice a default at the bottom. It's always good to have one in place in case none of your conditions match.

Also notice the break keyword. Without it the code block does not stop at the end and will "fall through" to the next block. This can be handy sometimes but not in your case.