CODEWITHSUNDEEP
php
Tigran
Tigran

Reputation: 1057

Can this PHP code structure be improved?

I have class that stores data about network packet:

var $from_ip;
    var $to_ip;
    var $from_port;
    var $to_port;
    var $tcp_length;
    var $tcp_stream_text;
    var $tcp_stream;
    var $tcp_sequence_dec;

And now, I am creating function to print it:

function Print_data()
{
    echo "<table>";
        echo "<tr><td>";
        echo "From IP:";
        echo "</td><td>";
        echo $this->from_ip;
        echo "</td></tr>";

        echo "<tr><td>";
        echo "To IP:";
        echo "</td><td>";
        echo $this->to_ip;
        echo "</td></tr>";


    echo "</table>";

}

Is there any way to omptimize it? As you may notice there will be a lot of same lines. (Exept, may be write a function to show it). But what about vars?

Upvotes: 0

Views: 90

Answers (5)

user2707535
user2707535

Reputation: 85

Maybe not the fastest, but i use something kinda like this for converting objects to assoc. arrays:

public function printObject($object)
{
    $reflectionClass = new ReflectionClass(get_class($object));        
    foreach ($reflectionClass->getProperties() as $property)
    {
        $property->setAccessible(true);
        echo "Prop: {$property->getName()}, Value: {$property->getValue($object)} \n";            
        $property->setAccessible(false);
    }
}

Upvotes: 0

hansW
hansW

Reputation: 44

As What does PHP keyword 'var' do? says, var is deprecated.

Here s the solution I propose,

<?php

class TCP {
    protected $from_ip;
    protected $to_ip;
    protected $from_port;
    protected $to_port;
    protected $tcp_length;
    protected $tcp_stream_text;
    protected $tcp_stream;
    protected $tcp_sequence_dec;

    private $template = "<table>
        <tr>
            <td>From IP:</td>
            <td>%s</td>
        </tr>
        <tr>
            <td>To IP:</td>
            <td>%s</td>
        </tr>
    </table>";

    static public function render_template() {
        return sprintf(self::$template, self::$from_ip, self::$to_ip);
    }
}

print TCP::render_template();

Upvotes: 1

Sterling Archer
Sterling Archer

Reputation: 22435

Try using an array! Use the associate array to set the keys to whatever you want, I just referenced the variable name. And the values are simply the ones you have now.

$data = array(
    "To IP" => $to_ip,
    "From IP" => $from_port,
    "To Port" => $to_port,
    "TCP Length" => $tcp_length,
    "TCP Stream Text" => $tcp_stream_text,
    "TCP Stream" => $tcp_stream,
    "TCP Sequence Dec" => $tcp_sequence_dec
);

And then iterate! Use a foreach loop to get the key and value, and boom. Much cleaner

function Print_data() {
    echo "<table>";
    foreach ($this->data as $key => $value) {
        echo "<tr><td>$key</td><td>$value</td></tr>";
    }
    echo "</table>";
}

Upvotes: 1

Matt Williamson
Matt Williamson

Reputation: 40243

This ought to work for your class

var $from_ip, $to_ip, $from_port, $to_port, $tcp_length, $tcp_stream_text, $tcp_stream, $tcp_sequence_dec;



function Print_data()
{
    echo <<<EOF
        <table>
            <tr><td>
            From IP:
            </td><td>
            $this->from_ip
            </td></tr>

            <tr><td>
            To IP:
            </td><td>
            $this->to_ip
            </td></tr>
        </table>
    EOF;

}

Upvotes: 0

Leng
Leng

Reputation: 2998

Your variables don't need the var on them, but if they're in a class, I recommend defining their access level (private, public, protected, etc):

private $from_ip;
private $to_ip;
private $from_port;
private $to_port;
private $tcp_length;
private $tcp_stream_text;
private $tcp_stream;
private $tcp_sequence_dec;

And this is how I would (personally) write the Print_data() function:

function Print_data() {
    ?>
    <table>
        <tr>
            <td>From IP:</td>
            <td><?php echo $this->from_ip ?></td>
        </tr>
        <tr>
            <td>To IP:</td>
            <td><?php echo $this->to_ip ?></td>
        </tr>
    </table>
    <?php
}

Upvotes: 1

Related Questions