MP3 Sharing

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
DS_
Forum Newbie
Posts: 3
Joined: Thu Jan 07, 2010 12:49 pm

MP3 Sharing

Post by DS_ »

Hello,

I'm currently attempting to learn php and I've gotten to a point where I am able to build functional scripts but I'm still looking for more.

I'm trying to improve the quality of my scripts and learn new things so that I can become better programmer.

I've been hunting around the internet looking for advice on how to become a better programmer as I'm aware that programming can't be mastered just by reading some books (although It certainly helps).

I read some where that it's beneficial to take part in a community and allow other coders to read your code and offer their advice, so by following that suggestion I've found myself here.

My script isn't complete but it is working -- I'd just like to hear your suggestions for improvement and what to learn/do next.

Thanks, Dan.
Attachments
empeethree.zip
(5.97 KiB) Downloaded 316 times
DS_
Forum Newbie
Posts: 3
Joined: Thu Jan 07, 2010 12:49 pm

Re: MP3 Sharing

Post by DS_ »

pickle | Please use [ code=php ], [ code=text ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.


I just had a look through other members that have posted code in a zip file and it seems that they have not got any replies. I understand that it's a load of hassle.

list_files.php

Code: Select all

include 'config.php';
include 'functions.php';
include 'db_connect.php';
 
 
$page = $_GET['page']; //get page number
$action = $_GET['action'];
 
if ($action == "list"){
    if (!isset($page)){$page = 0;} //if no page number set 0
    if (isset($page)){
        $query = "SELECT * from files LIMIT " . $page * $maxresult . "," . $maxresult;
        if ($result = mysqli_query($link,$query)){
            while ($row = mysqli_fetch_assoc($result)){
                echo "<a href=\"" . $_SERVER[’PHP_SELF’] . "?action=listen&uid=".$row['uid']."\">".$row['filename']."</a><br />";
            }
            mysqli_free_result($result);
        }
        $query = "SELECT * from files";
        if ($result = mysqli_query($link,$query)){
            $pagecount = round(mysqli_num_rows($result) / $maxresult,0,PHP_ROUND_HALF_DOWN);
            mysqli_free_result($result);
            for ($i = 0; $i <= $pagecount; $i++){
                if ($i != $page){
                    echo "<a href=\"". $_SERVER[’PHP_SELF’] . "?action=$action&page=$i\">$i</a> - ";
                } else {
                    echo "<b>$i</b> - ";
                }
            }
        }
    }
} elseif ($action == "listen"){
    $uid = $_GET['uid'];
    $query = "SELECT * from files WHERE uid='$uid'";
    if ($result = mysqli_query($link,$query)){
    
        $row = mysqli_fetch_assoc($result);
        
        echo $row['filename']."<BR />";
        echo "<object type=\"application/x-shockwave-flash\" data=\"player_mp3.swf\" width=\"200\" height=\"20\">"
             ."<param name=\"movie\" value=\"player_mp3.swf\" />"
             ."<param name=\"bgcolor\" value=\"#000000\" />"
             ."<param name=\"FlashVars\" value=\"mp3=http://". $_SERVER['HTTP_HOST'] ."$curdir$mp3directory" . $row['location'] . "\" />"
             ."</object>";
        mysqli_free_result($result);
    }
}
include 'db_close.php';
?> 
<a href="javascript&#058;history.back()">Go back</a>
process_upload.php

Code: Select all

<?php
include 'config.php';
include 'functions.php';
set_time_limit(0);
 
 
 
//Check for .mp3 extension
if (substr($_FILES['userfile']['name'],strlen($_FILES['userfile']['name']) - 4,strlen($_FILES['userfile']['name'])) == ".mp3"){
    
    $subdirectory = 1;
    $uploaddir    = $base_path . $mp3directory . $subdirectory . DIRECTORY_SLASH;
    $uploadname   = randomString(24) . ".mp3";
    
    while(countDir($uploaddir) >= 10){
        if (is_dir($uploaddir) == true){
            if (countDir($uploaddir) >= 10){
                $subdirectory++;
                $uploaddir = $base_path . $mp3directory . $subdirectory . DIRECTORY_SLASH;
                if(!is_dir($uploaddir)){mkdir($uploaddir);}
                }
            }
            $uploaddir = $base_path . $mp3directory . $subdirectory . DIRECTORY_SLASH;
        }
        
    if (move_uploaded_file($_FILES['userfile']['tmp_name'], $uploaddir . $uploadname)){
        echo "Success!\n";
        include 'db_connect.php';
        addFile($link,str_replace(".mp3","",$_FILES['userfile']['name']),"/$subdirectory/$uploadname");
        include 'db_close.php';
        createZip($uploaddir . $uploadname,$_FILES['userfile']['name']);
    } else {
        echo "Fail!\n";
        echo $_FILES['userfile']['error'];
    }
    
} else {
    echo "Invalid File Extension<BR />";
}
   
functions.php

Code: Select all

<?php
// General Functions
function randomString($length){
    $characters = array("a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z",1,2,3,4,5,6,7,8,9,0);
    for ($i = 0; $i < $length; $i++){
        $random_string .= $characters[rand(0,sizeof($characters))];
    }
    return $random_string;
}
 
function countDir($directory){
    $files = scandir($directory);
    foreach($files as $file){
        if (!($file == ".") && !($file == "..")){ 
            $count++;
        }
    }
    return $count;
}
 
//Database Functions
function addFile($link,$filename,$location){
    if ($result = mysqli_query($link,"INSERT INTO files (uid,filename,location,date) VALUES('','$filename','$location','')")){
    }
    
}
function createZip($mp3path,$filename){
    date_default_timezone_set('UTC');
    $zip = new ZipArchive;
    $zipdir = $_SERVER['DOCUMENT_ROOT'] . dirname($_SERVER['SCRIPT_NAME']) . "/zip/" . date('dm') . "/";
    if(!is_dir($zipdir)){mkdir($zipdir);}
    $res = $zip->open($zipdir . str_replace(".mp3",".zip",$filename), ZipArchive::CREATE);
    if ($res === true) {
        $zip->addFile($mp3path,$filename);
        $zip->setArchiveComment('Downloaded using empeethree.com');
        $zip->close();
        echo 'ok';
    } else {
        echo 'failed';
    }
}
 
 
 
?>
config.php

Code: Select all

<?php
//config file
define("MAX_FILE_DIR",1000);
define("SQL_HOST","localhost");
define("SQL_USER","root");
define("SQL_PASS","");
define("SQL_DB","empeethree");
define("DIRECTORY_SLASH","/");
$maxresult = 10;
$subdirectory = 1; //default subdirectory number
$mp3directory = "/mp3/"; //script directory
$curdir = dirname($_SERVER['SCRIPT_NAME']);
$base_path = $_SERVER['DOCUMENT_ROOT'] . $curdir;
//phpinfo();
 
?>

pickle | Please use [ code=php ], [ code=text ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: MP3 Sharing

Post by pickle »

Gotta admit - that's why I skipped over it.

That's a lot of code & you haven't really explained what you want to do. Getting "how do I make this better" advice is going to be tough - "better" is rather subjective, and improving code isn't nearly as fun as fixing, so you'll likely not get as many responses.

Quickly looking over the code, I did notice a few things:
  • In your config file, why don't you define everything as a constant? Why use some constants and others just plain variables? I'd use everything as a constant, or at the very least, change the variables to all capitals - which makes them still just ordinary variables, but at least when you see an all capitalized variable, you know it's from the config file
  • Your randomString() function can be optimized a bit. A string can be treated somewhat like an array, so rather than have a $characters array, use a characters string. The length/size of $characters doesn't change, so only call sizeof() once, rather than repeatedly in each iteration of the for loop.
  • You've got a lot of spaghetti code in there - something which usually happens with people new to the language. The commonly accepted practice is to separate code that displays stuff (display logic), and code that does the work behind the scenes (business logic). For example, rather than echoing "Success!" or "Fail!" when trying to move the file, store than in a variable that is then displayed at the end. On more complex programs, you'll appreciate this technique because you won't have to sort through thousands of lines of code to figure out where "Success" is echoed.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
DS_
Forum Newbie
Posts: 3
Joined: Thu Jan 07, 2010 12:49 pm

Re: MP3 Sharing

Post by DS_ »

Thanks for the advice but be warned, I'll be back!
Post Reply