0
\$\begingroup\$

I am working on a code to see if a video already exists. The problem is that I have to check multiple sections. The code is working but it should not be this big, I think, and it takes a long time to load.

Is there an easier way to do this?

$xmlurl7b = 'http://mysite.me/library/sections/2/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl1b = 'http://mysite.me/library/sections/5/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl2b = 'http://mysite.me/library/sections/7/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl3b = 'http://mysite.me/library/sections/9/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl4b = 'http://mysite.me/library/sections/10/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl5b = 'http://mysite.me/library/sections/11/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl6b = 'http://mysite.me/library/sections/13/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl8 = 'http://mysite.me/library/sections/3/all?title=' . $movie->title() . '&HndJk7Sd'; 
$xmlurl9 = 'http://mysite.me/library/sections/8/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl7 = 'http://mysite.me/library/sections/2/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl1 = 'http://mysite.me/library/sections/5/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl2 = 'http://mysite.me/library/sections/7/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl3 = 'http://mysite.me/library/sections/9/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl4 = 'http://mysite.me/library/sections/10/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl5 = 'http://mysite.me/library/sections/11/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl6 = 'http://mysite.me/library/sections/13/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$achxml8 = simplexml_load_file($xmlurl8);
$achxml9 = simplexml_load_file($xmlurl9);
$achxml1 = simplexml_load_file($xmlurl1);
$achxml2 = simplexml_load_file($xmlurl2);
$achxml3 = simplexml_load_file($xmlurl3);
$achxml4 = simplexml_load_file($xmlurl4);
$achxml5 = simplexml_load_file($xmlurl5);
$achxml6 = simplexml_load_file($xmlurl6);
$achxml7 = simplexml_load_file($xmlurl7);
$achxml1b = simplexml_load_file($xmlurl1b);
$achxml2b = simplexml_load_file($xmlurl2b);
$achxml3b = simplexml_load_file($xmlurl3b);
$achxml4b = simplexml_load_file($xmlurl4b);
$achxml5b = simplexml_load_file($xmlurl5b);
$achxml6b = simplexml_load_file($xmlurl6b);
$achxml7b = simplexml_load_file($xmlurl7b);
if( $achxml1->Video && ($achxml1->Video != '') 
OR $achxml2->Video && ($achxml2->Video != '') 
OR $achxml3->Video && ($achxml3->Video != '') 
OR $achxml4->Video && ($achxml4->Video != '') 
OR $achxml5->Video && ($achxml5->Video != '') 
OR $achxml6->Video && ($achxml6->Video != '') 
OR $achxml7->Video && ($achxml7->Video != '')
OR $achxml8->Directory && ($achxml8->Directory != '')
OR $achxml9->Directory && ($achxml9->Directory != '')
OR $achxml1b->Video && ($achxml1b->Video != '') 
OR $achxml2b->Video && ($achxml2b->Video != '') 
OR $achxml3b->Video && ($achxml3b->Video != '') 
OR $achxml4b->Video && ($achxml4b->Video != '') 
OR $achxml5b->Video && ($achxml5b->Video != '') 
OR $achxml6b->Video && ($achxml6b->Video != '') 
OR $achxml7b->Video && ($achxml7b->Video != '')
 ) {
echo 'Exists';
} else {
echo 'New'; 
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for posting your code. It smells. It can definitely use a Code Review! \$\endgroup\$ – Mast Jun 4 '15 at 12:06
  • 2
    \$\begingroup\$ The code needs improvement, but it's hard to show you how to generalize the code since it's not obvious what it's doing, or why it's doing it. For example, where do those URLs come from? \$\endgroup\$ – 200_success Jun 4 '15 at 12:09
  • \$\begingroup\$ If you're making requests to the server for specific sections, then logic would dictate that you should make a specific request for this type of thing. \$\endgroup\$ – Neil Jun 4 '15 at 12:14
-1
\$\begingroup\$

Try this approach: instead of using an if condition with many terms OR-ed together, which is hard to read and error prone, use a loop.

$sources = [
    [
        'url'  => 'http://mysite.me/library/sections/2/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/5/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/7/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/9/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/10/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/11/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/13/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/3/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Directory'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/8/all?title=' . $movie->title() . '&HndJk7Sd',
        'node' => 'Directory'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/2/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/5/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/7/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/9/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/10/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/11/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ],
    [
        'url'  => 'http://mysite.me/library/sections/13/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd',
        'node' => 'Video'
    ]
];

$exists = false;
foreach ($sources as $source) {
    $xml = simplexml_load_file($source['url']);

    if (isset($xml->{$source['node']}) && $xml->{$source['node']} != '') {
        $exists = true;
        break;
    }
}

if ($exists) {
    echo 'Exists';
} else {
    echo 'New';
}
\$\endgroup\$
0
\$\begingroup\$

This is a giant mess! Here's what you should do to start cleaning it up.

Code organization

  • Everything you are doing is quite repetitive with minimal variations. Start by constructing an array of each section number you need to inspect for title and each section number you need to inspect by guid. You can then iterate through these systematically without writing everything out.
  • Turn these two strings into constants you define at the top of your file '%3Flang%3Dnl&HndJk7Sd' and '&HndJk7Sd'. SImilarly does $movie->title() return the same value each time you call it? If so, move this value into a constant too.
  • I would then do a loop of a kind like this (pseudocode):

    for each filename in possible files{ video = simplexml_load_file(filename) if(video->Video && video->Video!='') { echo 'Exists' gracefully exit script? } //If script has not exited file doesn't exist echo 'New'; Note that this general organization means you stop as soon as you find a file. The way you have set up your code now, you load all directories but you might have actually found the video in the very first place you checked.

PHP language features

  • Any reason the standard function file_exists() isn't applicable in some way? Not sure why you seem to have all these special functions just to check if a file exists?
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.