Search code examples
phpmysqlperformancerefactoringcode-readability

php refactoring and better coding for php?


i have this process that is the heart of my app, that im creating, but for some reason i feel like its the worst way to do it(instinct) , and i wanted to see if thier is something wrong with this process, and am i approaching it in a bad way! p.s. the code works fine, just refactoring problem.

the process is:

users go to homepage, they see thier latest activities, by other site memebers(home.php),

 //function to bring the latest activities from database
   $results=function getUserUpdates($_SESSION['user_id'];


while($row = mysql_fetch_array($results))

      {
//another function to format the activities in a social stream
          echo formatUpdate($row['user_note'],$row['dt'],$row['picture'],$row['username'],$row['id'],$row['reply_id'],$row['reply_name'],$row['votes_up'],$row['votes_down']);


      }

i have put the function codes in pastie.

formatUpdate function http://pastie.org/1213958

getUserUpdates function http://pastie.org/1213962

EDIT both functions are from different files they are included in home.php, formatUpdate from functions.php getUserUpdates from queries.php


Solution

  • Firstly, I think you mean:

    $results = getUserUpdates($_SESSION['user_id']);
    

    In your getUserUpdates() function there is a redundant branch:

    if ($username == $_SESSION['u_name']){
        // return something
    }
    
    if ($username != $_SESSION['u_name']){
        // return something else
    }
    

    You don't need the second if statement as any code run at that point will only be run if $username != $_SESSION['u_name'].

    In my opinion, it's usually better not to have different functions directly echoing HTML up the stack (such as echoVote()). It's preferred to have functions return data and have the original caller echo it. This allows the caller to perform additional data massaging if desired.

    Other than that, your code is fetching data, looping through and acting on the results which is pretty much standard fare.

    I think your instinct is to be a little too harsh on yourself ;) There are improvements to be made but it's certainly not the worst way to do anything.