0
\$\begingroup\$

I am displaying the data from a json file onto a website, with recursive and iterative tables side by side. i have the iterative code working, but I have no idea on how to make it recursive.

Here is the code:

    $dataArray = json_decode($data, true);
            if(is_array($dataArray)){ //first level
                $result = "<table class='arrayTable'>";
                    foreach($dataArray as $key=>$val){ //second level
                        $result.="<tr><td class ='key'>".$key.": </td><td class ='value'>";
                        if(gettype($val)=="array"){
                            $result .= "<table class='arrayTable'>";
                            foreach($val as $subkey=>$subval){ //third level
                                $result.="<tr><td class ='key2'>".$subkey.": </td><td class ='value2'>";
                                if(gettype($subval)=="array"){
                                    $result .= "<table class='arrayTable'>";
                                    foreach($subval as $lowkey=>$lowval){ //fourth level
                                        $result.="<tr><td class ='key3'>".$lowkey.": </td><td class ='value3'>";
                                        if(gettype($lowval)!="array"){
                                            $result.=$lowval."</td></tr>";
                                        }else{
                                            $result.="</td></tr>";
                                        }
                                    }
                                    $result.="</table>";
                                }else{
                                    $result.=$subval."</td></tr>";
                                }
                            }
                            $result.="</table>";
                        }else{
                        $result.=$val."</td></tr>";
                        }
                    }
                $result.= "</table>";
            }else{
                echo "Not a valid format!";
            }
            return $result;
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review where we review working code you have written and make suggestions on how to improve that code. The part of the question about a recursive implementation is off-topic, the reason given will usually be `code not yet written or not working as expected'. \$\endgroup\$ – pacmaninbw Jun 27 at 3:46
  • \$\begingroup\$ A code review should be just that -- only analytical feedback. A complete code refactor can take much more time to write, test, and explain. If you have tried to refactor your script, but failed, then you have a Stack Overflow question. @Sam \$\endgroup\$ – mickmackusa Jun 28 at 10:35
  • \$\begingroup\$ @mickmackusa My question was removed from Stack Overflow, and the people there said to move it here. \$\endgroup\$ – Samatar Mohamed Jun 29 at 17:57
1
\$\begingroup\$
  1. Your code is a pain to read, please add early returns.
  2. To go with the point above, please follow PSR-12.
  3. iterative tables side by side. Your code creates another table within a , so it's no exactly side by side.

Here is how I refactored your code:

  1. I completed step 1 and 2.
  2. I have broken everything up into methods:
<?php

namespace App;

class DisplayArrayTable
{
    protected $result = '';

    /**
     * @param mixed $value
     */
    protected function setupMeaningFullName1(string $key, $data): void
    {
        $this->result .= "<tr><td class ='key'>{$key}: </td><td class ='value'>";

        if (!is_array($data)) {
            $this->result .= "{$data}</td></tr>";

            return;
        }

        $this->result .= "<table class='arrayTable'>";

        foreach ($data as $key => $value) {
            $this->setupMeaningFullName2($key, $value);
        }

        $this->result .= "</table>";
    }

    /**
     * @param mixed $value
     */
    protected function setupMeaningFullName2(string $key, $data): void
    {
        $this->result .= "<tr><td class ='key2'>{$key}: </td><td class ='value2'>";

        if (!is_array($data)) {
            $this->result .= "{$data}</td></tr>";

            return;
        }

        $this->result .= "<table class='arrayTable'>";

        foreach ($data as $key => $value) {
            $this->setupMeaningFullName2($key, $value);
        }

        $this->result .= "</table>";
    }

    /**
     * @param mixed $value
     */
    protected function setupMeaningFullName3(string $key, $data): void
    {
        $this->result .= "<tr><td class ='key3'>{$key}: </td><td class ='value3'>";

        if (!is_array($data)) {
            $this->result .= "{$data}</td></tr>";

            return;
        }

        $this->result .= "</td></tr>";
    }

    public function handle(array $data): string
    {
        foreach ($data as $key => $value) {
            $this->setupMeaningFullName1($key, $value);
        }

        return "<table class='arrayTable'>{$this->result}</table>";
    }
}
  1. I have identified the pattern and further refactored your code
<?php

namespace App;

class DisplayArrayTable
{
    const MAX_ITERATIONS = 3;
    protected $currentIteration = 1;
    protected $result = '';

    /**
     * @param mixed $value
     */
    protected function iteration(string $key, $data): void
    {
        $this->result .= "<tr><td class ='key'>{$key}: </td><td class ='value'>";

        if (!is_array($data)) {
            $this->result .= "{$data}</td></tr>";

            return;
        }

        if ($this->currentIteration === static::MAX_ITERATIONS) {
            $this->result .= '</td></tr>';
            
            return;
        }

        $this->result .= '<table class="arrayTable">';

        foreach ($data as $key => $value) {
            $this->currentIteration++;

            $this->setupMeaningFullName2($key, $value);
        }

        $this->result .= '</table>';
    }

    public function handle(array $data): string
    {
        foreach ($data as $key => $value) {
            $this->iteration($key, $value);
        }

        return "<table class='arrayTable'>{$this->result}</table>";
    }
}
  1. Performed further refactoring to improve iteration method
<?php

namespace App;

class DisplayArrayTable
{
    const MAX_ITERATIONS = 3;
    protected $currentIteration = 1;
    protected $result = '';

    public function handle(array $data): string
    {
        foreach ($data as $key => $value) {
            $this->iteration($key, $value);
        }

        return "<table class='arrayTable'>{$this->result}</table>";
    }

    /**
     * @param mixed $value
     */
    protected function iteration(string $key, $data): void
    {
        $this->result .= "<tr>
            <td class ='key'>{$key}: </td>
            <td class ='value'>";

        if (!is_array($data)) {
            $this->result .= "{$data}</td></tr>";

            return;
        }

        if ($this->currentIteration === static::MAX_ITERATIONS) {
            $this->result .= '</td></tr>';
            
            return;
        }

        $this->setupNewTable($data);
    }

    protected function setupNewTable(array $data): void 
    { 
        $this->result .= '<table class="arrayTable">';

        foreach ($data as $key => $value) {
            $this->currentIteration++;

            $this->iteration($key, $value);
        }

        $this->result .= '</table>';
    }
}

So, all you need to do now is call this call and pass in a valid array

use App\DisplayArrayTable;

...

$dataArray = json_decode($data, true);

if ($dataArray && is_array($dataArray)) {
   $html = (new DisplayArrayTable)->handle($dataArray);
}
| improve this answer | |
\$\endgroup\$

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