2
\$\begingroup\$

I've got an XML file with some info about hosts connected to my home network and I would like to create objects containing that info in a neat way, so that I can make some nice front-end application with that data later on.

This is how I do it:

  • Loading the XML
  • Encoding it into JSON
  • Making an array from the JSON
  • Making a simpler array from the last array
  • Creates objects from the array
  • Done!

My question to you is: Is this the way to do it, or is there some better way? Am I missing something obvious? Is this the way to do it the "OOP" way? I'm trying to learn this, so all constructive feedback is welcome!

class Host {
    public $ipv4, $mac;

    public function __construct($ipv4, $mac){
        $this->ipv4 = $ipv4;
        $this->mac = $mac;
    }
}

class PagesController extends Controller
{
    public function index(){

        // Get xml with hosts that are connected to local network
        $xml = file_get_contents("C:/test.xml");
        $xml = simplexml_load_string($xml, "SimpleXMLElement", LIBXML_NOCDATA);
        $json = json_encode($xml);
        $array = json_decode($json,TRUE);

        // put all hosts in separate array (to make it easier to work with when creating an object further down) 
        foreach ($array['host'] as $hostKey => $hostValue) {
            foreach ($hostValue['address'] as $addressKey => $addressValue) {
                foreach ($addressValue['@attributes'] as $attributeKey => $attributeValue) {
                    $hosts['host'.$hostKey]['address'.$addressKey][$attributeKey] = $attributeValue;
                }
            }
        }

        // Create object from array created earlier
        foreach ($hosts as $hostKey => $hostValue) {
            if(isset($hostValue)){
                foreach ($hostValue as $addressKey => $addressValue) {
                    if(isset($addressValue)){
                        if(isset($addressValue['addrtype'])){
                            // ipv4 ? 
                            if($addressValue['addrtype'] == 'ipv4'){
                                $ipv4 = $addressValue['addr'];  
                            // or mac-address?
                            } elseif($addressValue['addrtype'] == 'mac'){
                                $mac = $addressValue['addr'];
                            }
                        }
                    }
                }
            }
            // Skapa ett objekt som heter host+$hostKey
            $hosts[$hostKey] = new Host($ipv4, $mac);
        }

Note: The reason for the PagesController class and the index() method is because I'm using the Laravel framework.

The xml-file is generated from nmap which I plan to run every five minutes or so. This is what the xml looks like:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE nmaprun>
<?xml-stylesheet href="file:///usr/bin/../share/nmap/nmap.xsl" type="text/xsl"?>
<!-- Nmap 7.70 scan initiated Fri Jan 18 19:54:05 2019 as: nmap -sP -oX test.xml 192.168.1.1/24 -->
<nmaprun scanner="nmap" args="nmap -sP -oX test.xml 192.168.1.1/24" start="1547837645" startstr="Fri Jan 18 19:54:05 2019" version="7.70" xmloutputversion="1.04">
    <verbose level="0"/>
    <debugging level="0"/>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.1" addrtype="ipv4"/>
        <address addr="E8:FC:AF:7B:42:46" addrtype="mac" vendor="Netgear"/>
        <hostnames>
        </hostnames>
        <times srtt="597" rttvar="5000" to="100000"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.2" addrtype="ipv4"/>
        <address addr="6C:AD:F8:C5:BB:65" addrtype="mac" vendor="AzureWave Technology"/>
        <hostnames>
        </hostnames>
        <times srtt="47811" rttvar="47811" to="239055"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.4" addrtype="ipv4"/>
        <address addr="04:69:F8:9C:F5:1F" addrtype="mac" vendor="Apple"/>
        <hostnames>
        </hostnames>
        <times srtt="50599" rttvar="50599" to="252995"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.5" addrtype="ipv4"/>
        <address addr="C8:69:CD:C8:44:A7" addrtype="mac" vendor="Apple"/>
        <hostnames>
        </hostnames>
        <times srtt="55214" rttvar="55214" to="276070"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.7" addrtype="ipv4"/>
        <address addr="68:D9:3C:CC:FA:1D" addrtype="mac" vendor="Apple"/>
        <hostnames>
        </hostnames>
        <times srtt="78890" rttvar="73429" to="372606"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.9" addrtype="ipv4"/>
        <address addr="14:DA:E9:51:3F:CC" addrtype="mac" vendor="Asustek Computer"/>
        <hostnames>
        </hostnames>
        <times srtt="467" rttvar="5000" to="100000"/>
    </host>
    <host>
        <status state="up" reason="arp-response" reason_ttl="0"/>
        <address addr="192.168.1.200" addrtype="ipv4"/>
        <address addr="00:15:5D:01:05:00" addrtype="mac" vendor="Microsoft"/>
        <hostnames>
        </hostnames>
        <times srtt="453" rttvar="5000" to="100000"/>
    </host>
    <runstats>
        <finished time="1547837648" timestr="Fri Jan 18 19:54:08 2019" elapsed="3.12" summary="Nmap done at Fri Jan 18 19:54:08 2019; 256 IP addresses (8 hosts up) scanned in 3.12 seconds" exit="success"/>
        <hosts up="8" down="248" total="256"/>
    </runstats>
</nmaprun>
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Where does the XML file come from? Is it generated by a service/product? Would it be possible for you to edit your post to include a sample (with names/addresses altered to protect sensitive values, if necessary)? \$\endgroup\$ Commented Jan 21, 2019 at 22:03
  • \$\begingroup\$ Thank you! I've added the xml and some info about its origin in the original post. I've used nmap to generate the file and for now it is static, but I'm planning on putting it to run on a schedule. Heard that there is something called cron-jobs, so thats probably what I'm going to use. You think that is a good idea or should I use something else? \$\endgroup\$ Commented Jan 22, 2019 at 11:42

1 Answer 1

Reset to default
2
\$\begingroup\$

Addressing your questions

Is this the way to do it, or is there some better way? Am I missing something obvious?

I'm reminded of this highly upvoted SO answer to How do you parse and process HTML/XML in PHP?. Based on the information there, you could choose a different tactic, like using the DOM API by utilizing thr DOMDocument class. This would allow you to do the following:

  • remove the steps that encode and decode the data as JSON
  • simplify access of node attributes

And the step "Making a simpler array from the last array" seems excessive as well - why not just create the hosts as soon as the appropriate data is parsed/found?

The code below has one less level of foreach loops because it simply checks if the address node has attributes for addrtype and addr. It also eliminates the second set of foreach statements and creates the host objects as soon as the information is available.

libxml_use_internal_errors(true); //ignore invalid HTML tag warnings
$dom = new DOMDocument();
$dom->loadHTMLFile('C:/test.xml');
foreach($dom->getElementsByTagName('host') as $hostKey => $host) {
    $hostAttributes = array();
    foreach($host->getElementsByTagName('address') as $adressKey => $addressValue) {
        if ($addressValue->getAttribute('addrtype') && $addressValue->getAttribute('addr')) {
            $hostAttributes[$addressValue->getAttribute('addrtype')] = $addressValue->getAttribute('addr');
        }
    }
    if (array_key_exists('ipv4', $hostAttributes) || array_key_exists('mac', $hostAttributes) {
        $hosts[$hostKey] = new Host($hostAttributes['ipv4'], $hostAttributes['mac']);
    }            
}

Is this the way to do it the "OOP" way?

If you mean the parsing techniques, one might say that your approach is to parse JSON objects with arrays of nested children and then create Host objects - seems fine.

General review points

If you don't decide to simplify the code with an approach like above, consider these critiques of your existing code.

The value of the variables $ipv4 and $mac persist across iterations of the foreach loops, which means that if a host had only one type of address, then a previous value may be used unintentionally...

The following lines can be simplified:

if(isset($addressValue)){
      if(isset($addressValue['addrtype'])){

Why not combine those into a single line combined with the logical AND operator (i.e. &&)? I could see a reason not to do so if there was an else case defined immediately after one of those but that is not the case.

if(isset($addressValue) && isset($addressValue['addrtype'])){
\$\endgroup\$
4
  • \$\begingroup\$ Thanks alot for a your answer! I tried your way with the DOMDocument and I think it looks alot better. I also think that it will make it easier for me later on when I'm about to add more properties to the objects. Your answer was spot on! \$\endgroup\$ Commented Jan 23, 2019 at 19:33
  • 1
    \$\begingroup\$ P.S would give an upvote, but I've got too low reputation for that \$\endgroup\$ Commented Jan 23, 2019 at 19:34
  • \$\begingroup\$ Okay thanks - if you earn that vote up privilege then it would be appreciated if you come back to this :) \$\endgroup\$ Commented Jan 23, 2019 at 19:37
  • \$\begingroup\$ I sure will! :) \$\endgroup\$ Commented Jan 23, 2019 at 19:49

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.