anthrt Posted December 9, 2007 Share Posted December 9, 2007 I have a script which parses user's iTunes library files, in XML format and logs it to a DB so it can be displayed in their forum profile. Before it gets entered in the DB, some cleaning up/verification is done to the elements, i.e. song title, genre etc. I've written a function to do this and put it in a seperate functions file with some other ones. One of the verifications is to check that the current item in the XML file is not a movie file, this appears to be the only one of the verification steps that actually works. Also, the function (should) display a message which says why the track wasn't accepted, e.g. not long enough, missing song title etc. This message isn't displayed on the page. It also tries to fill in some non-essential data if it's missing, so if there's no genre defined for the track it replaces it with 'unknown'. Anyway, here's the function function checkrow(&$song, &$msg) { //global $song; // global $msg; $song['Artist'] = utf8_decode($song['Artist']); $song['Name'] = utf8_decode($song['Name']); if(strpos($song['Kind'], 'movie') || strpos($song['Kind'], 'video')) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - movie file)</span><br>\n"; return false; } else { if($song['Total Time'] <= 30000) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - too short (30s))</span><br>\n"; return false; } else { if(empty($song['Name'])) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - no song title!)</span><br>\n"; return false; } else { if(empty($song['Artist']) || !isset($song['Artist']) || strlen($song['Artist']) <= 1) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - no artist name!)</span><br>\n"; return false; } else { if(empty($song['Equalizer']) || !isset($song['Equalizer'])) { $song['Equalizer'] = 'Unknown'; $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Equalizer missing, setting equalizer to 'unknown')<br>\n"; return true; } else { if(empty($song['Genre']) || !isset($song['Genre'])) { $song['Genre'] = 'Unknown'; $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Genre missing, setting genre to 'unknown') <br>\n"; return true; } else { $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Added)<br>\n"; return true; } } } } } } } The problem is that the $song['Equalizer'] and $song['Genre'] variables aren't being rewritten with "unknown" if they're empty. All the other verification checks work, the messages and $song['Name'] gets shown etc. I'm not too sure about the references in the function parameters, I was just messing around trying to get it to work. Here's how the function is called foreach ($songs as $song) { $song_name = mysql_real_escape_string($song['Name']); $song_artist = mysql_real_escape_string($song['Artist']); $song_album = mysql_real_escape_string($song['Album']); $song_genre = mysql_real_escape_string($song['Genre']); $song_kind = mysql_real_escape_string($song['Kind']); $song_size = mysql_real_escape_string($song['Size']); $song_total_time = mysql_real_escape_string($song['Total Time']); $song_year = mysql_real_escape_string($song['Year']); $song_bit_rate = mysql_real_escape_string($song['Bit Rate']); $song_sample_rate = mysql_real_escape_string($song['Sample Rate']); $song_equalizer = mysql_real_escape_string($song['Equalizer']); $song_play_count = mysql_real_escape_string($song['Play Count']); $song_location = mysql_real_escape_string($song['Location']); if(checkrow($song, $msg)) { //$query = $db->sql_query("query"); echo $msg; } else { echo $msg; } } Quote Link to comment Share on other sites More sharing options...
anthrt Posted December 10, 2007 Author Share Posted December 10, 2007 Doesn't anyone have any idea how I can fix this? Quote Link to comment Share on other sites More sharing options...
btherl Posted December 10, 2007 Share Posted December 10, 2007 The function finishes after one of the conditions is met, because of the return. Could that be causing the problem? Two more things - First, your formatting makes it very difficult to read the logic of your code! Second, you can write like this: if (something) { blah } elseif (somethingelse) { blah } elseif (somethingelse) { blah } I think that style is much easier to read than the continual nesting you have used in your code. Quote Link to comment Share on other sites More sharing options...
anthrt Posted December 10, 2007 Author Share Posted December 10, 2007 Yeah, that looked pretty crappy. Here it is cleaned up function checkrow(&$song, &$msg) { $song['Artist'] = utf8_decode($song['Artist']); $song['Name'] = utf8_decode($song['Name']); if(strpos($song['Kind'], 'movie') || strpos($song['Kind'], 'video')) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - movie file)</span><br>\n"; return false; } elseif($song['Total Time'] <= 30000) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - too short (30s))</span><br>\n"; return false; } elseif(empty($song['Name'])) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - no song title!) </span><br>\n"; return false; } elseif(empty($song['Artist']) || !isset($song['Artist']) || strlen($song['Artist']) <= 1) { $msg = "<span class='error'>" . $song['Artist'] . ' - ' . $song['Name'] . " (Not added - no artist name!)</span><br>\n"; return false; } // Below doesn't work elseif(empty($song['Equalizer']) || !isset($song['Equalizer'])) { $song['Equalizer'] = 'Unknown'; $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Equalizer missing, setting equalizer to 'unknown')<br>\n"; return true; } // Below doesn't work elseif(empty($song['Genre']) || !isset($song['Genre'])) { $song['Genre'] = 'Unknown'; $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Genre missing, setting genre to 'unknown') <br>\n"; return true; } else { $msg = $song['Artist'] . ' - ' . $song['Name'] . " (Added)<br>\n"; return true; } } Anyway, back to the problem. The function finishes after one of the conditions is met, because of the return. Could that be causing the problem? No, if it returns false it won't even reach the equalizer and genre variables. What I mean is, if anything above $song['Equalizer'] returns false, it won't matter about changing the equalizer variable because it won't reach it anyway. If none of them return false, but $song['Equalizer'] is empty, it should replace/set the variable to "Unknown" instead of it being blank. This isn't happening, and neither is it for the one below it. Quote Link to comment Share on other sites More sharing options...
btherl Posted December 10, 2007 Share Posted December 10, 2007 So even the first one, the Equalizer check, is not working? Ok I understand the problem. I notice that you mysql_real_escape_string() your variables before you call check_row() .. could that be part of the problem? Even if $song is altered by check_row(), those changes will not be reflected in your $song_* variables Quote Link to comment Share on other sites More sharing options...
anthrt Posted December 10, 2007 Author Share Posted December 10, 2007 Heh, good call. I never thought twice about that. Alas, it still doesn't seem to be working. I thought that would have been it, but I guess not. foreach ($songs as $song) { $song['Name'] = mysql_real_escape_string($song['Name']); $song['Artist'] = mysql_real_escape_string($song['Artist']); $song['Album'] = mysql_real_escape_string($song['Album']); $song['Genre'] = mysql_real_escape_string($song['Genre']); $song['Kind'] = mysql_real_escape_string($song['Kind']); $song['Size'] = mysql_real_escape_string($song['Size']); $song['Total Time'] = mysql_real_escape_string($song['Total Time']); $song['Year'] = mysql_real_escape_string($song['Year']); $song['Bit Rate'] = mysql_real_escape_string($song['Bit Rate']); $song['Sample Rate'] = mysql_real_escape_string($song['Sample Rate']); $song['Equalizer'] = mysql_real_escape_string($song['Equalizer']); $song['Play Count'] = mysql_real_escape_string($song['Play Count']); $song['Location'] = mysql_real_escape_string($song['Location']); if(checkrow($song, $msg)) { $sql = "query"; $query = $db->sql_query($sql); echo $msg; } The function is still the same as above. Quote Link to comment Share on other sites More sharing options...
btherl Posted December 11, 2007 Share Posted December 11, 2007 Ok, if this was my program, I would debug it by putting print statements inside each of the "if" conditions that I suspected. Then you can see if they are being executed. If they are not executed, then either the condition is not being matched, or the condition is not even being checked (due to the program logic). After that you will know exactly which code is executed. Next is either fixing the code that is executed, or making it so the code you want is executed (after finding out it wasn't when it should be). For that, I would use var_dump($song) to see what's inside the $song, and ask myself why it is not matching the condition? It may be something subtle like the "blank" values having some invisible character in them, and therefore not matching the empty() condition. Quote Link to comment Share on other sites More sharing options...
anthrt Posted December 11, 2007 Author Share Posted December 11, 2007 There are messages that are displayed, it's the $msg variable in the function. They are showing so obviously the condition is being met. Quote Link to comment Share on other sites More sharing options...
btherl Posted December 11, 2007 Share Posted December 11, 2007 Ok, that means that your code is being executed but is not doing what it should be doing. Can you post your query as well? Oh, a good thing to do would be to var_dump($song) after the call to checkrow(). That will let you know if the changes are really being applied to $song Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.