7
\$\begingroup\$

I need to get familiar with web programming, so I decided to write up a fairly simple PHP page that accepts GET requests and serves back an image of the Mandelbrot set... as a table of colored cells.

Examples:

Mandelbrot Set Form Example

and

Nicer Example

I'm sending the response as a table because I didn't want to get into using image libraries yet, so I wanted to try doing it entirely using HTML, and I couldn't think of a simpler way.

Basically how it works is, you indicate where in the Set you want to view by specifying the "Bounds" fields, the size of the view and table, the aspect ratio (as a decimal), then nine colored fields that decide how the image will be colored.

The main logic is essentially a port of my C code from last year, and the coloring idea is pulled from a project I did several years ago in Clojure.

I'm looking for suggestions about anything. I haven't used HTML, PHP, or CSS is years, and I was never great with them to begin with. There's a lot here, so I don't expect super thorough reviews, but anything would be appreciated.

Specifically though:

  • The function new_color_f() is a disaster. Basically, I want to be able to produce a function that has some parameters already set, and I'm doing that by closing over the parameters of the enclosing function. It's long and ugly though, and is made worse by PHP's use syntax. Any suggestions there would be appreciated.

  • The function defaulting_get() seems like a smell as well. I need to account for the GET data being missing, and potentially empty. That lead to using a bizarre mix of ?? and ?: operators though.

Anything except:

  • Yes, this is stupid to do in PHP the way I am, and it's also stupid to do this using an HTML table. It causes issues with the input form being wiped every entry, and also causes incredible browser lag when receiving larger tables. I wanted to get some experience with PHP thought before I try something more complicated, and I've always loved this project. Really, I should be doing this in JavaScript, or using AJAX to request images instead of using a form.

The server is running PHP 7.4 for reference.


index.php

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Mandelbrot Generator</title>
    <style>            
        .display_table {
            font-size: 1px;
            border-spacing: 0;
        }
      
        .input_form {
            display: table;
        }
        
        .input_row {
            display: table-row;
        }
        
        .input_row label, .input_row input {
            display: table-cell;
        }
        
        .red_input input, .green_input input, .blue_input input {
            border-width: 5px;
        }
        
        .red_input input {
            border-color: red;
        }

        .green_input input {
            border-color: lightgreen;
        }

        .blue_input input {
            border-color: blue;
        }
        
    </style>
</head>
<body>
    <form method="get" class="input_form">
        <div class="input">
            <div class="location_input">
                <div class="input_row">
                    <label>Lower Real Bound</label>
                    <input name="lreal" type="number" min="-2" max="2" step="any">
                </div>
                <div class="input_row">
                    <label>Lower Imaginary Bound</label>
                    <input name="limag" type="number" min="-2" max="2" step="any">
                </div>
        
                <div class="input_row">
                    <label>View Width</label>
                    <input name="vwidth" type="number" min="0" max="4" step="any">
                </div>
                <div class="input_row">
                    <label>Pixels Wide</label>
                    <input name="pwidth" type="number" min="0" max="1000" step="any">
                </div>
        
                <div class="input_row">
                    <label>Aspect Ratio</label>
                    <input name="aratio" type="number" min="0" max="2" step="any">
                </div>
            </div>

            <div class="color_input">
                <div class="red_input">
                    <div class="input_row">
                        <label>Real</label>
                        <input name="rr" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Imaginary</label>
                        <input name="rim" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Iters</label>
                        <input name="rit" type="number" min="-1000" max="1000" step="any">
                    </div>
                </div>

                <div class="green_input">
                    <div class="input_row">
                        <label>Real</label>
                        <input name="gr" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Imaginary</label>
                        <input name="gim" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Iters</label>
                        <input name="git" type="number" min="-1000" max="1000" step="any">
                    </div>
                </div>

                <div class="blue_input">
                    <div class="input_row">
                        <label>Real</label>
                        <input name="br" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Imaginary</label>
                        <input name="bim" type="number" min="-1000" max="1000" step="any">
                    </div>
                    <div class="input_row">
                        <label>Iters</label>
                        <input name="bit" type="number" min="-1000" max="1000" step="any">
                    </div>
                </div>
                
            </div>
        </div>

        <button type="submit">Submit</button>
    </form>
 
</body>
</html>

<?php
    include "display.php";
    
    const DEF_LOW_REAL = -2;
    const DEF_LOW_IMAG = -2;
    const DEF_VIEW_WIDTH = 4;
    const DEF_PIX_WIDTH = 100;
    const DEF_ASP_RATIO = 1;
    const DEF_COLOR_MULT = 2;
    
    function defaulting_get($key, $default) {
        return ($_GET[$key] ?? $default) ?: $default;
    }
    
    $low_real = defaulting_get("lreal", DEF_LOW_REAL);
    $low_imag = defaulting_get("limag", DEF_LOW_IMAG);
    $view_width = defaulting_get("vwidth", DEF_VIEW_WIDTH);
    $pixels_wide = defaulting_get("pwidth", DEF_PIX_WIDTH);
    $aspect_ratio = defaulting_get("aratio", DEF_ASP_RATIO);

    $view_height = $view_width / $aspect_ratio;
    $high_real = $low_real + $view_height;
    $high_imag = $low_imag + $view_width;
    $pixels_high = $pixels_wide / $aspect_ratio;
    
    $color_f = new_color_f(
        defaulting_get("rr", DEF_COLOR_MULT),
        defaulting_get("rim", DEF_COLOR_MULT),
        defaulting_get("rit", DEF_COLOR_MULT),
        defaulting_get("gr", DEF_COLOR_MULT),
        defaulting_get("gim", DEF_COLOR_MULT),
        defaulting_get("git", DEF_COLOR_MULT),
        defaulting_get("br", DEF_COLOR_MULT),
        defaulting_get("bim", DEF_COLOR_MULT),
        defaulting_get("bit", DEF_COLOR_MULT),
    );

    emit_mandelbrot_view(
        $color_f,
        $low_real,
        $high_real,
        $low_imag,
        $high_imag,
        $pixels_wide,
        $pixels_high);

display.php

<?php
    include "iteration.php";
    
    const COLOR_MAX = (2 << 7) - 1;
    
    function clamp_color($n) {
        return max(0, min(COLOR_MAX, $n));
    }
    
    function checked_produce_rgb($red, $green, $blue) {
        $c_red = clamp_color($red);
        $c_green = clamp_color($green);
        $c_blue = clamp_color(($blue));
        return "rgb($c_red, $c_green, $c_blue)";
    }
    
    function new_color_f($red_real, $red_imag, $red_iter,
                         $gre_real, $gre_imag, $gre_iter,
                         $blu_real, $blu_imag, $blu_iter) {
        
        $color_func = function($real, $imag, $iters) use
                              ($red_real, $red_imag, $red_iter,
                               $gre_real, $gre_imag, $gre_iter,
                               $blu_real, $blu_imag, $blu_iter) {
            
            return checked_produce_rgb(
                $real * $red_real + $imag * $red_imag + $iters * $red_iter,
                $real * $gre_real + $imag * $gre_imag + $iters * $gre_iter,
                $real * $blu_real + $imag * $blu_imag + $iters * $blu_iter
            );
        };
        
        return $color_func;
    }

    function produce_pixel($color) {
        return "<td style='background: $color; color: $color'>_</td>";
    }
    
    function emit_mandelbrot_view($color_f,
                                  $lower_real,
                                  $upper_real,
                                  $lower_imag,
                                  $upper_imag,
                                  $pixels_wide,
                                  $pixels_high) {
        
        $real_step =  ($upper_real - $lower_real) / $pixels_wide;
        $imag_step = ($upper_imag - $lower_imag) / $pixels_high;
        
        echo "<table class='display_table'>";
        for ($imag = $lower_imag; $imag <= $upper_imag; $imag += $imag_step) {
            echo "<tr>";
            for ($real = $lower_real; $real <= $upper_real; $real += $real_step) {
                $iters = test_point([$real, $imag]);
                $color = $color_f($real, $imag, $iters);
                
                echo produce_pixel($color);
            }
            echo "</tr>";
        }
        echo "</table>";
    }

iteration.php

<?php
    // Make mutative?
    
    const STD_MAX_ITERS = 200;
    const STD_INF_LIMIT = 2;

    function square_complex($complex) {
        [$real, $imag] = $complex;
        return [($real * $real) - ($imag * $imag),
                2 * $real * $imag];
    }
    
    function mandelbrot_iteration($init_complex, $curr_complex) {
        [$i_real, $i_imag] = $init_complex;
        $sqrd = square_complex($curr_complex);
        
        $sqrd[0] += $i_real;
        $sqrd[1] += $i_imag;
        
        return $sqrd;
    }
    
    function is_under_limit($complex, $inf_limit) {
        [$real, $imag] = $complex;
        return ($real * $real) + ($imag * $imag) <= ($inf_limit * $inf_limit);
    }
    
    function test_point($initial_complex,
                        $max_iters = STD_MAX_ITERS,
                        $inf_limit = STD_INF_LIMIT) {
        $current = $initial_complex;
    
        $i = 0;
        for (; $i < $max_iters && is_under_limit($current, $inf_limit); $i++) {
            $current = mandelbrot_iteration($initial_complex, $current);
        }
        
        return $i;
    }
\$\endgroup\$
0
3
\$\begingroup\$

User Interface

I noticed that if I entered values into the form and then submitted it, the values were not persisted after the next page load. It would be nice to have the values persisted in the form inputs. This is easy to achieve since the form is rendered by a PHP page.

Because the PHP code towards the latter half of index.php comes after the HTML is closed, the HTML generated for the table is technically outside the HTML document:

    </form>

  </body>
</html>

<table class='display_table'>

Most modern browsers would display the table as if it were inside the <body> tag but it isn't technically a valid structure. Using https://validator.w3.org/ with the generated source shows this output:

validation errors

A common convention is to have most of the PHP code at the top of the file - especially useful if there is functionality that modifies HTTP headers e.g. calls to header() since headers cannot be sent after HTML has been emitted.

Remember that header() must be called before any actual output is sent, either by normal HTML tags, blank lines in a file, or from PHP.1

The HTML generated from the PHP calls (e.g. calls to emit_mandelbrot_view() which would need to return the generated HTML instead of calling echo directly) can be stored in a variable or emitted inline, optionally using the shortcut syntax for echo - i.e. <?=:

<?php
//setup PHP code here
?>
<!DOCTYPE html>
<html lang="en">
    <head>
        <!-- ... HTML headers here -->
    </head>
    <body>
        <form>
        <!-- ... HTML here -->
        </form>
         <?= emit_mandelbrot_view($color_f...)?>
    </body>
</html>

Code suggestions

  • new_color_f is a disaster. Basically, I want to be able to produce a function that has some parameters already set, and I'm doing that by closing over the parameters of the enclosing function. It's long and ugly though, and is made worse by PHP's use syntax. Any suggestions there would be appreciated.

Since you confirmed the server that the code runs on is using PHP 7.4 then the code can be updated to take advantage of newer features like arrow functions

Arrow functions support the same features as anonymous functions, except that using variables from the parent scope is always automatic.

This would allow the use statement to be eliminated in new_color_f().


The function range() can be used with foreach to simplify the nested loops within emit_mandelbrot_view():

for ($imag = $lower_imag; $imag <= $upper_imag; $imag += $imag_step) {
    echo "<tr>";
    for ($real = $lower_real; $real <= $upper_real; $real += $real_step) {

Can be simplified to:

foreach (range($lower_imag, $upper_imag, $imag_step) as $imag) {
    echo "<tr>";
    foreach (range($lower_real, $upper_real, $real_step) as $real) {

though given the amount of times that function is called it might not be advisable to add more function calls just to simplify the syntax.


For this block in index.php:

    $color_f = new_color_f(
        defaulting_get("rr", DEF_COLOR_MULT),
        defaulting_get("rim", DEF_COLOR_MULT),
        defaulting_get("rit", DEF_COLOR_MULT),
        defaulting_get("gr", DEF_COLOR_MULT),
        defaulting_get("gim", DEF_COLOR_MULT),
        defaulting_get("git", DEF_COLOR_MULT),
        defaulting_get("br", DEF_COLOR_MULT),
        defaulting_get("bim", DEF_COLOR_MULT),
        defaulting_get("bit", DEF_COLOR_MULT),
    );

Obviously there is a lot of duplication here. One simplification would be to put the query string keys into an array and loop over them, passing them to the function and storing the results in an array that can be spread out into a call to new_color_f().


In test_point() the iterator is set up outside the loop:

$i = 0;
for (; $i < $max_iters && is_under_limit($current, $inf_limit); $i++) {
       $current = mandelbrot_iteration($initial_complex, $current);
}

Unlike c code $i can be declared in the first statement of the for loop and still be visible outside the loop since variables aren't limited to a block but rather a function (if applicable). This isn't really explained well in the PHP documentation variable scope though there is a comment about it.

dodothedreamer at gmail dot com ¶ 8 years ago


Note that unlike Java and C++, variables declared inside blocks such as loops or if's, will also be recognized and accessible outside of the block, so:

<?php
for($j=0; $j<3; $j++)
{
     if($j == 1)
        $a = 4;
}
echo $a;
?>

Would print 4.


This constant definition looks like a C style optimization

const COLOR_MAX = (2 << 7) - 1; 

Correct me if I am wrong but I doubt there is any optimization over declaring this as 255:

const COLOR_MAX = 255;

P.S. I asked about that constant declaration on Stack Overflow. From the responses so far it just seems to be personal preference as far as how the value is declared. If you prefer using the bit-shifted syntax then feel free to do so though be forewarned that if you were to work on a team with others that could come up as a question in a code review.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. I'm not sure how I didn't realize that I was creating the table outside of the document. What would be the typical way of setting this up? Putting everything from include "display.php"; to the end of $color_f = . . . at the very top, then put the call to emit_mandelbrot_view inside its own set of <?php tags at the bottom of body (or wherever I want it in the document)? \$\endgroup\$ – Carcigenicate Sep 5 '20 at 14:15
1
\$\begingroup\$

Types

It makes the code easier to understand if you specify types for all function signatures. PHP will check types if specified at runtime whenever entering and exiting a function, and when writing to a class property, helping you to detect and find errors much more quickly, especially if using an IDE or static analysis tool. They also help communicate the intent of your code to make it easier to add more to it.

Some example changes to add types:

In index.php, change function defaulting_get($key, $default) { to function defaulting_get(string $key, int $default): int {

In display.php, change function clamp_color($n) { to function clamp_color(int $n): int {

and

    function new_color_f($red_real, $red_imag, $red_iter,
                         $gre_real, $gre_imag, $gre_iter,
                         $blu_real, $blu_imag, $blu_iter) {

to

    /** @return \Closure(int, int, int): string */
    function new_color_f(int $red_real, int $red_imag, int $red_iter,
                         int $gre_real, int $gre_imag, int $gre_iter,
                         int $blu_real, int $blu_imag, int $blu_iter): \Closure {

I'm not sure if I've got the type of that closure quite right in the docblock.

I would do this for every single function.

Avoiding use

You can avoid the need for use by taking advantage of the short closure syntax introduced in PHP 7.4:

        /** @return \Closure(int, int, int): string */
        function new_color_f(int $red_real, int $red_imag, int $red_iter,
                            int $gre_real, int $gre_imag, int $gre_iter,
                            int $blu_real, int $blu_imag, int $blu_iter): \Closure {
        return fn(int $real, int $imag, int $iters): string => 
              checked_produce_rgb(
                $real * $red_real + $imag * $red_imag + $iters * $red_iter,
                $real * $gre_real + $imag * $gre_imag + $iters * $gre_iter,
                $real * $blu_real + $imag * $blu_imag + $iters * $blu_iter
            );
        };
    }

PHP automatically encloses any variables mentioned after the => in the short closure.

Classes and autoloading

Modern idiomatic PHP, particularly for larger applications, tends to have almost all code in the form of classes, even if the application isn't intended to be object-oriented. One advantage of using classes is that PHP's autoloading system can load them on demand. This is not available for free functions. Static functions on classes can be conveniently used in place of free functions.

Replacing all the free functions with static class functions will also make it possible to mark any functions used only within the class that declares them as private, which should make it much easier to understand code flow.

The typical way to use autoloading is to set up the project with Composer, organize the classes one per file, following PSR-4 rules for the location and name of each file, and make Composer generate an autoload.php file. This can be called with require_once from index.php or any other entry point. Any classes needed after that will be automatically loaded if they can be found.

I would also make new_color_f return an object instead of a closure, and rename it accordingly. This object can have private fields instead enclosed variables, and a getRGB public function.

\$\endgroup\$

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.