1
\$\begingroup\$

I have been working on a class to use Reflection to interrogate other PHP classes and interfaces, what I want to know from anyone with more experience of this is, is there anything else I can add, or is there a better way of doing the things I am trying to do.

In essence I want to be able to use the interrogator to tell me everything it can about a class I give it, interfaces, parents, methods etc etc. When working on large multi-file projects this would give you an idea of the structure of the code and where to find things you need or want to change, eg you change a method in a parent only to find it was overridden by a child class, this should tell you if methods etc are overridden.

thoughts / comments etc welcome.

<?php

/* Test / Dummy classes and interfaces for testing */

interface i1
{
  const interface_i1_version = '1.0.0';

  public function fred();
}

interface i2 extends i1
{
  const interface_i2_version = '2.0.0';
}

interface i3
{
  const interface_i3_version = '3.0.0';
}

class c1
{
  const class_c1_version = '1.0.0';

  private $test1;
  var $test2;
  var $test3 = 'fred';

  public function __construct()
    {
    }

  public function foo()
    {
    }
}

class c2 extends c1 implements i3
{
  const class_c2_version = '2.0.0';

  public function bar()
    {
    }
}

class c3 extends c2 implements i1,i2
{
  const class_c3_version = '3.0.0';

  public static $Test = "sub";

  public function foo($bob = 1)
    {
    }

  public function fred()
    {
    }
}

class c4 extends c3
{
  const class_c4_version = '3.0.0';

  public static $Test = "bob";

  public function foo($bob = 1)
    {
    }
}

/* The real class to do the work */

class interogate
{
  private $constants = Array();

  private function get_type($ro)
    {
      return implode(' ', Reflection::getModifierNames($ro->getModifiers()));
    }

  private function in_array_r($needle, $haystack, $strict = true)
    {
      foreach ($haystack as $item)
        {
          if (($strict ? $item === $needle : $item == $needle) || (is_array($item) && $this->in_array_r($needle, $item, $strict)))
            {
              return true;
            }
        }
      return false;
    }

  private function check_parent($name, $parent)
    {
      foreach ($parent as $p)
        {
          if ($name == $p['name'])
            {
              return $p;
            }
        }
      return false;
    }

  private function update_global_constants($name, $location)
    {
      foreach ($this->constants as $constant)
        {
          if ($constant['name'] === $name)
            {
              return;
            }
        }
      $constant = Array('name' => $name, 'location' => $location);
      $this->constants[] = $constant;
    }

  private function get_global_constants($name)
    {
      foreach ($this->constants as $constant)
        {
          if ($constant['name'] === $name)
            {
              return ($constant['location']);
            }
        }
      return (false);
    }

  private function constants_from_interfaces($interface_name)
    {
      $results = Array();

      try
        {
          $ro = new ReflectionClass($interface_name);
        }
      catch (ReflectionException $re)
        {
          return($results);
        }

      foreach ($ro->getConstants() as $name => $value)
        {
          $this->update_global_constants($name, $interface_name);

          $location = $this->get_global_constants($name);

          $results[] = Array('name' => $name, 'value' => $value, 'location' => $location);
        }
      return ($results);

    }

  private function interogate_interfaces($ro)
    {
      $results = Array();

      foreach ($ro->getInterfaceNames() as $in)
        {
          if (!($this->in_array_r($in, $results)))
            {
              $constants = $this->constants_from_interfaces($in);
              $results[] = Array('name' => $in, 'constants' => $constants);
            }
        }
      asort($results);
      return ($results);
    }

  private function interogate_statics($ro)
    {
      $results = Array();

      foreach ($ro->getStaticProperties() as $name => $value)
        {
          $results[] = Array('name' => $name, 'value' => $value);
        }
      return ($results);
    }

  private function interogate_constants($ro, $class_name)
    {
      $results = Array();

      foreach ($ro->getConstants() as $name => $value)
        {
          $this->update_global_constants($name, $class_name);

          $location = $this->get_global_constants($name);

          $results[] = Array('name' => $name, 'value' => $value, 'location' => $location);
        }
      return ($results);
    }

  private function interogate_properties($ro)
    {
      $results = Array();

      foreach ($ro->getProperties() as $p)
        {
          $name = $p->name;

          $results[] = Array('name' => $p->name, 'value' => 'todo');
        }
      return ($results);
    }

  public function interogate_methods($ro, $name, $parent, $interface)
    {
      $results              = Array();
      $results['local']     = Array();
      $results['inherited'] = Array();

      foreach ($ro->getMethods() as $m)
        {
          $local     = $this->check_parent($m->name, (isset($parent['methods']['local']))?$parent['methods']['local']:Array());
          $inherited = $this->check_parent($m->name, (isset($parent['methods']['inherited']))?$parent['methods']['inherited']:Array());

          $overridden      = 0;
          $overridden_from = 0;
          if ((($local !== false) || ($inherited !== false)) && ($m->class == $name))
            {
              $overridden = 1;

              if ($local !== false)
                {
                  $overridden_from = $local['class'];
                }
              elseif ($inherited !== false)
                {
                  $overridden_from = $inherited['class'];
                }
            }

          if ($interface)
            {
              $location = 'local';
            }
          elseif ($m->class == $name)
            {
              $location = 'local';
            }
          else
            {
              $location = 'inherited';
            }

          $type = $this->get_type($m);

          $inheritable = 0;
          if (($m->isProtected()) || ($m->isPublic()))
            {
              $inheritable = 1;
            }

          if (($inheritable == 0) && ($location == 'inherited'))
            {
              // Skip ??
            }
          else
            {
              $parameters = Array();
              foreach ($m->getParameters() as $p)
                {
                  if ($p->isOptional())
                    {
                      $optional = 'Yes';
                      try
                        {
                          $default = $p->getDefaultValue();
                        }
                      catch (ReflectionException $re)
                        {
                          $default = 'Built In';
                        }
                    }
                  else
                    {
                      $optional = 'No';
                      $default = 'none';
                    }
                  $position = $p->getPosition();
                  $parameters[] = Array('name' => $p->name, 'optional' => $optional, 'default' => $default, 'position' => $position);
                }
              $results[$location][] = Array('name' => $m->name, 'class' => $m->class, 'overridden' => $overridden, 'overridden_from' => $overridden_from, 'inheritable' => $inheritable, 'modifier' => $type, 'parameters' => $parameters);
            }
        }

      return ($results);
    }

  public function interogate_object($name)
    {
      $results = Array();

      try
        {
          $rc1 = new ReflectionClass($name);
        }
      catch (ReflectionException $re)
        {
          return($results);
        }

      if ($rc1->isInterface())
        {
          $type = 'interface';
          $interface = 1;
        }
      else
        {
          $type = 'class';
          $interface = 0;
        }

      $results['name']    = $name;
      $results['type']    = $type;

      $filename   = $rc1->getFileName();
      $start_line = $rc1->getStartLine();
      $end_line   = $rc1->getEndLine();

      $results['filename'] = ($filename == FALSE)?'Unknown':$filename;
      $results['details']  = (($start_line == FALSE) || ($end_line == FALSE))?'Unknown':'Between lines: ' . $start_line . ' and ' . $end_line;

      $parent = (array) $rc1->getParentClass();
      if (array_key_exists('name', $parent))
        {
          $parent = $parent['name'];
          $results['parent'] = $this->interogate_object($parent);
        }

      $results['interfaces'] = $this->interogate_interfaces($rc1);
      $results['methods'] = $this->interogate_methods($rc1, $name, (isset($results['parent']))?$results['parent']:Array(), $interface);

      $results['statics'] = $this->interogate_statics($rc1);
      $results['constants'] = $this->interogate_constants($rc1, $name);
      $results['properties'] = $this->interogate_properties($rc1);

      return ($results);
    }

}

$i = new interogate();

//$results = $i->interogate_object('ReflectionClass');
$results = $i->interogate_object('c4');

print_r($results);

?>

I am not sure if anyone else has done this already, or if I am making some bad assumptions, but this is my first play with Reflection so this is as much a learning exercise as anything else.

The completed class (once complete of course), will be available for free (release GPL v3) for anyone to make use of.

\$\endgroup\$
  • 1
    \$\begingroup\$ If you are doing this project for the experience, that's fine. But if you are doing it for those reasons you gave, then a much better, and easier, solution would be to adopt a good IDE. For instance, my favorite IDE, Netbeans, does all of this natively via the Navigator pane and "find usages" tool. As for determining if a method is overriden in a child? Well, a gray circle next to the parent method will tell you that, and a green circle next to the child method will confirm it. Ctrl+Click to follow to source, etc... I'll look over the code and give you my opinion in a few. \$\endgroup\$ – mseancole Aug 31 '12 at 13:47
  • \$\begingroup\$ I am fine with using a decent IDE but I like to write code to benefit not just myself but other people if possible. So a good IDE would benefit me directly but what about others? This is a valid point of course, but I like a challenge so I want to push this and see how far I can go with it :) \$\endgroup\$ – Tim 'Wolf' Gurney Aug 31 '12 at 13:51
  • \$\begingroup\$ The idea is to have a decent output rather than a var_dump for the final version, so you can do full interrogation of a class, as I say mostly a coding exercise to learn something new. \$\endgroup\$ – Tim 'Wolf' Gurney Aug 31 '12 at 13:53
1
\$\begingroup\$

There are a few things that stood out. I understand this is prototype code, but some of the things I am seeing shouldn't even be present in prototypes. So, I'm just going to treat this as a normal review and point out everything, just in case. As for helping everyone, a good IDE still does this and more. So it is more likely to be installed before some random report generating API. Please do not take offense, I am merely explaining it as others will see it. The benefits of an IDE, that allows seamless integration into the current environment, far outweigh that of a simple "report" that distracts from the code the user is attempting to write. There would have to be some reason or feature that would drive someone to use this API over an IDE. And then you would be better off developing it as a plugin for your favorite IDE. Of course, a couple of things you said make me think that this is mostly just a challenge for you, so that is fine, I just wanted to make sure you understood that this is not likely to become the next PHPUnit. Then again, I've been known to be wrong. If you want an accurate assessment, then I would request input from the community on various forums to determine if there is a need. But in the mean time...

Your interfaces could use some work. Again, I understand this might be because you are just prototyping it, but even prototype interfaces should be better fleshed out. If all you are going to do is add constants to them, then you might as well not use interfaces. An interface is supposed to inform the person implementing it what methods and properties are available. As it stands, you would be better off just assigning those constants in the class you wanted to use them in, it would save the overhead at least. Its good to use interfaces, but only if they are being used properly.

Those old-fashioned properties, var, while not deprecated any more, are likely to become deprecated again before too long. It is the old way of declaring a public property. If your version of PHP still requires this syntax, then it is safe to say that you should upgrade it. I'll demonstrate the proper way below, I'll also sneak in the multideclaration syntax.

public
    $test2,
    $test3 = 'fred'
;

What is $ro? Be descriptive with your variables, especially if you are planning on sharing this with others as you mentioned. Sure I can guess what it is supposed to be, but if it were any deeper or more abstracted, this might become a challenge. Besides, I shouldn't have to guess. Well written code is self documenting code. Always remove the uncertainty. Not only will this help others use your code, but it will make it easier for you to return to your code later.

Why do you have a Ternary in an if statement? For one, I didn't even know that was possible; I mean I guess I knew it was, but I've never thought about doing it. Two, why would you do it? You are only making your code more difficult to read. Even after I abstracted the ternary operation from this statement it is still hard to read. This is usually a sign that you are trying to do too much. Try to see how you can simplify it. Here's the ternary abstracted.

$stringCompare = $strict ? $item === $needle : $item == $needle;
if( $stringCompare || ( is_array( $item ) && $this->in_array_r( $needle, $item, $strict ) ) ) {

And here is a simplified version.

private function in_array_r( $needle, $haystack, $strict = TRUE ) {
    if( ! is_array( $haystack ) ) {
        return $strict ? $item === $needle : $item == $needle;
    }

    foreach( $haystack AS $item ) {
        if( $this->in_array_r( $needle, $item, $strict ) ) {
            return true;
        }
    }

    return false;
}

A better notation for foreach loops is to use the plural as the array and the singular as element. This follows that "self documenting" rule and is pretty intuitive to follow. Using single letter variables will only confuse the matter. The only single letter variables should be incrementals or throw away variables, example $i or $x.

foreach( $parents AS $parent ) {

You should not use try/catch blocks to control the flow of your program. They are meant to be used for error checking. Either throw a new exception, or do something with the current one. If the program fails, you should inform the user, just returning a value is misleading.

Why are your ternary operations getting harder and harder to read? Be consistent in your style. Also, ternary statements are still statements. A good convention to follow states that there should only be one statement per line, so ternary should declared on its own line and defined to a variable. This helps with legibility, and ensures that your lines do not become excessively long.

$localMethods = isset( $parent[ 'methods' ] [ 'local' ] ) ? $parent[ 'methods' ] [ 'local' ] : array();
$local = $this->check_parent( $m->name, $localMethods );

Of course, even some ternary statements are too long and would be better written as plain if statements. Ternary should not be used if it makes your code harder to read.

$localMethods = array();
if( isset( $parent[ 'methods' ] [ 'local' ] ) {
    $localMethods = $parent[ 'methods' ] [ 'local' ];
}

Don't use excessive parenthesis. They are not necessary for every comparison, only for those that would return a different value because of precedence. The more parenthesis there are, the harder the statement is to follow. So only use them where they are necessary.

if( ( $local !== false || $inherited !== false ) && $m->class == $name ) {

And again, simplify your statements. Don't do a comparison more than once.

if( $m->class == $name ) {
    $overridden = 1;

    if( $local !== FALSE ) {
        $overridden_from = $local[ 'class' ];
    } else if( $inherited !== FALSE ) {
        $overridden_from = $inherited[ 'class' ];
    } else {
        $overridden = 0;
        $overridden_from = 0;
    }
}

Additionally, don't recreate booleans. If you have a variable that toggles between a TRUE/FALSE state, then use a boolean. A boolean does not always need to be a boolean, just for the FALSE state. That's why you always see === FALSE and rarely see === TRUE. The first is necessary, the second is redundant. For example, both $overridden and $overridden_from both have a FALSE state where their value is zero, and a TRUE state where their value contains the overriding method. The first state is misleading, the second state is natural. When I first saw those variables being declared with zero values, I thought you were setting up a counter to determine how many times it was being overridden. It wasn't until I read on that I figured out what you were trying to do. It is much more intuitive and natural to read $overridden = FALSE; rather than $overridden = 0;. Its also much easier to do a proper comparison.

PHP is very nice in that it does not care about whitespace. So your long array declarations, or method parameters, can be pushed to multiple lines. This makes your code much easier to read.

$parameters[] = array(
    'name'     => $p->name,
    'optional' => $optional,
    'default'  => $default,
    'position' => $position
);

Just a couple more things. First, your methods should be shorter. Those last couple were extremely large. I didn't read over them completely, so I can't describe how exactly, but keep in mind the Single Responsibility Principle, and you should be ok. Second, when you are appending a bunch of data onto an array, all at once, do so in a new array and then merge that array onto the old. For example:

$temp = array(
    'statics'    => $this->interogate_statics( $rc1 );
    'constants'  => $this->interogate_constants( $rc1, $name );
    'properties' => $this->interogate_properties( $rc1 );
);

$results = array_merge( $results, $temp );

Hope this helps

\$\endgroup\$
  • \$\begingroup\$ The opening paragraph of my post is the most important to read, I am looking for feedback on the logic of the code not the layout/style/format. This was a 30 min hack over lunch, so it will get cleaned up prior to release. I am more concerned that the code logic and functionality is right. \$\endgroup\$ – Tim 'Wolf' Gurney Aug 31 '12 at 17:16
  • \$\begingroup\$ @Tim'Wolf'Gurney: As is the opening paragraph of my post. In it I stated that I understood this and that even so there were some things that were concerning even in the prototyping stage. As such I reviewed the whole thing to ensure everything was covered. Is the logic sound? Asides from the things I pointed out, sure. Though, as I mentioned, I fail to see the benefits of such a class, so I cannot extrapolate from it and be more detailed. Can it be done better? Sure, but you'll actually have to write the code before I can do anything more. \$\endgroup\$ – mseancole Aug 31 '12 at 18:33
  • \$\begingroup\$ Ok that makes sense, and sorry if I sounded ungrateful before because I am not you gave a lot of useful feedback which I will take into account when I clean it up. As for the point of it? I doubt there is one to be honest other than a learning exercise. So thank you for the complete feedback it will be taken on board while I finish it off. \$\endgroup\$ – Tim 'Wolf' Gurney Sep 1 '12 at 10:04

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.