-44
\$\begingroup\$

Just wondering if there was someone out there that could offer help in optimizing my script which "derpifies" images.

<?php
/**
 * If you don't understand what this does
 * you have no hope as a programmer.
 *
 */
function usage ( )
{
  // Ahhh the marvels of echo, such a wonderful tool
  // yet so advanced that a beginner will tremble at the
  // word. Woe's me is spoken to the RAM with such care.
  echo "\n";
  echo "----------------------------\n";
  echo "> Derp.PHP is a CLI based   \n";
  echo "> image derpification tool. \n";
  echo "> Use only with:              \n";
  echo ">   jpeg, gif, or png files \n";
  echo ">                             \n";
  echo "> Usage:                    \n";
  echo ">   derp.php path/to/file   \n";
  echo "----------------------------\n";
  echo "\n";
}

/**
 * In this function we make sure the end user isn't
 * a total moran by checking weather we have access
 * to certain hacker libraries.
 *
 **/
function make_checks ( )
{
  # A beautiful built_in function that is necessary to
  # this programs inherited existence, such a lovely sight.
  if ( !function_exists('imagecopy') )
  {
    // Here we throw an error if we encounter a stupid user.
    // >2012
    // >Not having GD installed.
    //
    // ISHYGDDT.
    echo "\n";
    echo ">>> ERROR <<<\n";
    echo ">>> INSTALL THE GD LIBRARY\n";
    echo "\n";
    /*
      This is a boolean return. False essentially means
      0; TMYK.
    */
    return false;
  }
  else
    return true; # Note the lack of curly brackets around this
                 # line of code. Amazing technology these days.
}

/**
 * Stand back kiddies, this is how a real
 * programmer does things.
 *
 **/
function main ( $argv )
{
  // Enterprise usage of the count() function
  // is demonstrated here, take note of the
  // combined < and = symbol here, only a master
  // programmer could have achieved such an
  // operater without R(ing)TFM.
  if ( count($argv) <= 1 )
  {
    // An excellent call to an undoubtedly elegant
    // and not to mention genius display function.
    usage();
    exit; # Notice the multiple comment types in this file.
        # The mark of a true programming genius.
  }
  // Simple and elegant usage of the master make_checks() 
  // library checking function, what would be referred to 
  // by a Java programmer as a Factory(lmfao Java).
  if ( !make_checks() )
    exit; # Sweet succulent success. NOT(lmfao!1)!

  // The file to be derpafied.
  $file = $argv[1];
  # Only the technologically independent and savvy programmer
  # will be able to truly understand the genius of the below
  # statement; one that even William Shakespear could not have
  # written better.
  $type = ( substr($file, -4, 1) !== '.' ) // Can you see?
      ? substr($file, -4) // Can you see my genius.
      : substr($file, -3) // It's shining...
      ; // oh so bright. What a marvelous function, I
        // couldn't have done it better myself. Oh, wait.

  echo $type, "\n";
}

main( $argv );
\$\endgroup\$
  • 34
    \$\begingroup\$ You're not funny. :) Oh, and echo pathinfo($file)['extension']; // PHP 5.4 \$\endgroup\$ – Quentin Pradet Mar 20 '12 at 7:28
80
\$\begingroup\$

This script is a total of 99 lines. Of that, 49 lines are insulting, annoying, and ironic self-congratulating comments.

In general, comments are good. They help any future maintainer (including yourself when it's been 6 months since the last time you looked at the script) to understand what the code does because programmer time is extraordinarily valuable. Programmer time should be spent fixing the code--not trying to understand it, and helpful comments go a long way toward cutting down on the "trying to understand it" part of the program.

These comments are not funny, at least not in a "laughing with you" sort of way. And they are not constructive. And perhaps worst of all, they're not even consistent.


/**
 * In this function we make sure the end user isn't
 * a total moran by checking weather we have access
 * to certain hacker libraries.
 *
 **/

Here, we accuse the user of being a "moran" by checking the "weather" or something (though the function doesn't at all appear to be accessing any sort of weather web service). This comment might be better written as:

// Determine whether necessary libraries exist

This removes the misspellings and still accurately describes in plain-English what the goal of the function is.


// Here we throw an error if we encounter a stupid user.
// >2012
// >Not having GD installed.
//
// ISHYGDDT.

As a note from someone who has plenty of experience dealing with users, your best bet is to program for the lowest common denominator. A "stupid" user is still a user, and if you're not capable of writing programs that "stupid" users can use, then perhaps it is you who has no hope as a programmer.

This comment might be better written as:

// Alert the user that a necessary library is missing

I'd also argue that including some instructions on how to install the missing library would probably be helpful in your echo statements.


/**
 * Stand back kiddies, this is how a real
 * programmer does things.
 *
 **/

This comment is completely unhelpful and can be completely removed.


  // Enterprise usage of the count() function
  // is demonstrated here, take note of the
  // combined < and = symbol here, only a master
  // programmer could have achieved such an
  // operater without R(ing)TFM.

As a programmer, you have 3 options.

The first option is to develop your own programming language and write the manual for it, and then you don't have to read the manual. This is PHP; you didn't create it, so you're going to fall into one of the other two categories.

The second option is to read the manual. This is what the master programmers do. Manuals are boring and hard to read, but if you want to make the most out of a given programming language, you have to be familiar with the documentation for that language or... fall into the third category...

The third option is learning from other programmers who already read the manual, and while you could still be a "master" programmer and be in this category, you're no more "master" than those from whom you learn.


  # Only the technologically independent and savvy programmer
  # will be able to truly understand the genius of the below
  # statement; one that even William Shakespear could not have
  # written better.

Once again, another misspelling in the midst of self-congratulating the genius of a programmer who can't even spell-check.

The irony of this comment is astounding.

Let's first of all ignore the fact that you misspelled Shakespeare and ignore the fact that he was a playwright, not a programmer, so if you can't write code better than him, again, you're probably the one who has no hope at a programmer.

Instead, let's look at the comment which introduces a somewhat confusing ternary statement. The whole point of comments, again, is to make code more readable. Although, a true master programmer writes the executable code in a highly readable way as a starting point, and only needs comments to explain the bits of code that can't easily be written in a highly readable manner.

There's nothing that impressive about the ternary statement--it's certainly not worthy of self-congratulation. However, it'd be my guess that there's a better way of doing this in PHP (though I'm no PHP master, so maybe not). But if we feel the ternary statement is not so obvious to the average reader of our source code that a comment is necessary, then the appropriate sort of comment would be to clarify what the code is actually doing. Something more along the lines of this:

// Grabs the file extension

Also, by the way...

// oh so bright. What a marvelous function

It's not a function. It's an operator. It's called the ternary operator. The fact that it's called a ternary operator is something you could learn by reading the documentation.

\$\endgroup\$
  • 39
    \$\begingroup\$ I don't know weather to upvote or ... Eh, what the heck. +1. \$\endgroup\$ – RubberDuck Aug 19 '14 at 0:18
  • \$\begingroup\$ I can't believe you actually went to the trouble of reviewing that and the end result is good. (+1) \$\endgroup\$ – Marc.2377 Jun 15 at 20:48
46
\$\begingroup\$
sed 's/\(\/\/\|#\).*$//g' YOUR_FILENAME | sed '/^$/N;/^\s*$/D'

To clarify, that strips comments and multiple blank lines. Generally comments are a good idea. I would not remove them or create shorter names for functions and variables for performance. Any benefit there is minuscule beyond belief. Your comments trolled me into giving that answer, seriously though: remove your comments.

Ignoring the Comments - some real recommendations

Consistency makes things easier to read.

  1. Work out your maximum line length and use it (especially for the comments).
  2. Stop trolling me with mixed brace styles in if/else statements.

make_checks is not a very descriptive name for a function. It would be better as check_libraries.

Now, the bit of your code that actually does something:

$type = ( substr($file, -4, 1) !== '.' ) // Can you see?
     ? substr($file, -4) // Can you see my genius.
     : substr($file, -3) // It's shining...
     ; // oh so bright. What a marvelous function, I
       // couldn't have done it better myself. Oh, wait.

Should be rewritten:

$type = pathinfo($file, PATHINFO_EXTENSION);

In fact, the variables $file and $type are only used once (and hence shouldn't even be variables).

\$\endgroup\$
  • 12
    \$\begingroup\$ Hmm, it looks like you are a PHP guru. We could use one of your kind in our chat room. ;) \$\endgroup\$ – syb0rg Feb 14 '14 at 1:10

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.