2
\$\begingroup\$

After my previous question I revised the code to the code below. I am using PHP 7.0.33 and tried to accomplish PSR-12 as much as I could. In "condensed usage" in the docblock you can find a full copy-paste example.

Here is the code:

/**
 * This class aims to render a valid HTML form block
 *
 * @author julio
 * @date 1/5/2023
 *
 *
 * -- Condensed usage --
 *
$form = new Form([
    'name'=>"my_form",
    'action'=>"destination.php",
    'method'=>"post"
]);

$form->add_tag("fieldset");
$form->add_tag("legend");
$form->add_tag_attributes(['text'=>"My FLDST 1"]);

$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"my_text",
    'text'=>"My Text:"
]);
$form->add_tag("input");
$form->add_tag_attributes([
    'id'=>"my_text",
    'type'=>"text",
    'name'=>"my_text",
    'value'=>"", 'required'=>null
]);
$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"my_textarea",
    'text'=>"My Text Area:"
]);
$form->add_tag("textarea");
$form->add_tag_attributes([
    'id'=>"my_textarea",
    'name'=>"my_textarea",
    'cols'=>80,
    'rows'=>20,
    'text'=>"My textarea text"
]);

$form->add_tag("fieldset");

$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"my_check",
    'text'=>"My Check:"
]);
$form->add_tag("input");
$form->add_tag_attributes([
    'type'=>"checkbox",
    'name'=>"my_check",
    'value'=>"",
    'id'=>"my_check",
    'checked'=>true
]);

$form->add_tag("select");
$form->add_tag_attributes(['name'=>"my_select"]);
$form->add_tag_options(
    [
        "value11"=>"text11",
        "value12"=>"text12",
        "value13"=>"text13"
    ],
    ["value13"]
);
$form->add_tag("select");
$form->add_tag_attributes([
    'name'=>"my_other_select",
    'multiple'=>true
]);
$form->add_tag_options(
    [
        "value1"=> ["value11"=>"text11", "value12"=>"text12", "value13"=>"text13"],
        "value2"=> ["value21"=>"text21", "value22"=>"text22", "value23"=>"text23"],
        "value3"=> ["value31"=>"text31", "value32"=>"text32", "value33"=>"text33"],

    ],
    ["value11", "value22", "value33"]
);
$form->add_tag("fieldset");
$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"radio1",
    'text'=>"radio1"
]);
$form->add_tag("input");
$form->add_tag_attributes([
    'type'=>"radio",
    'id'=>"radio1",
    'name'=>"radio",
    'value'=>"radio1"
]);
$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"radio2",
    'text'=>"radio2"
]);
$form->add_tag("input");
$form->add_tag_attributes([
    'type'=>"radio",
    'id'=>"radio2",
    'name'=>"radio",
    'value'=>"radio2"
]);
$form->add_tag("label");
$form->add_tag_attributes([
    'for'=>"radio3",
    'text'=>"radio3"
]);
$form->add_tag("input");
$form->add_tag_attributes([
    'type'=>"radio",
    'id'=>"radio3",
    'name'=>"radio",
    'value'=>"radio3"
]);
$form->add_tag("fieldset");
$form->add_tag("input");
$form->add_tag_attributes([
    'type'=>"submit",
    'value'=>"enviar",
    'name'=>"enviar"
]);
echo $form->render(); // or $html = $form->render();
 *
 * -- Important --
 *
 * This code is written for PHP 7.0.33 so visibility, return types and some
 * other improvements are not available to me. However I tried to accomplish
 * PSR 12 as much as a I could.
 *
 * You will notice there are no escaping functions in use. This is intended
 * for a developer so any kind of validation is left for it.
 *
 * Thorough tests still need to be run but so long this class is working.
 *
 * -- Notes about my coding style --
 *
 * As much as I try to accomplish standards there are certain things I don't
 * like such as camel case for method names (and will ignore anything related
 * to this), using sprintf() for replacements or swith to `[]` instead of
 * `array()` (I use `array()` to declare multidimensional arrays and will
 * continue this way), just to name some of them.
 * They are not much and IMO they can be allowed.
 *
 * -- About this class --
 *
 * This class aims to render a valid code for HTML forms.
 * When this class is instantiated constructor takes an associative array
 * to set <form> attributes. Like this:
 *
 *     $form = new Form([
 *         "name" => "my_form",
 *         "action" => "destination.php",
 *         "method" => "post"
 *     ]);
 *
 * Those three attributes are mandatory. The others are optional.
 *
 * In general:
 *
 * parameters, when its type is array, are associative arrays, and
 *
 * mandatory attributes must be present and have a valid value, and
 *
 * if any optional attribute is set it must contain a non empty
 * string excepting boolean attributes such as `required`, `autofocus`,
 * `multiple` and `checked` (which can hold anything you want) or when the
 * attribute is `value`. When dealing with labels, textareas, fieldsets and
 * legends attribute `value` is named `text`, and
 *
 * custom or non standard attributes are not allowed and will be dismissed
 * with a user level warning (not implemented yet)
 *
 * To add a tag:
 *
 * $form->add_tag("tag_name");
 * $form->add_tag_attributes(["attr1"=>"value1", ...]);
 *
 * If intended tag is a select, options must be set:
 *
 * $form->add_tag_options(["opt1"=>"value1", ...]);
 *
 * If select tag has any pre-selected value:
 *
 * $form->add_tag_options(["opt1"=>"value1", ...], ["selected1", "selected2", ...]);
 *
 * This documentation is still in progress.
 */
class Form
{
    /** in here each valid tag is stored.
     * it is looped in render()
     * @var array
     */
    private $output = array();

    /**
     * used to hold tag name while building HTML
     * to reduce the number of times tag name should be passed as parameter
     * @var string
     */
    private $tag = "";

    /** used to build tag
     * @var string
     */
    private $tag_build = "";

    /**
     * flag for when a <select> is intended
     * @var boolean
     */
    private $select_options_required = false;

    /**
     * flag to check if <option>'s are set
     * @var boolean
     */
    private $select_options_set = false;

    // as I'm using PHP 7.0.33 visibility is not allowed for constants
    // all this constants should be private

    const FORM_TAGS = ['form', 'input', 'label', 'select', 'textarea', 'button', 'fieldset', 'legend',
        'datalist', 'output', 'option', 'optgroup'];
    const FORM_TAGS_TYPES = array(
        'input' => ['button', 'checkbox', 'color', 'date', 'datetime-local', 'email', 'file', 'hidden', 'image', 'month',
            'number', 'password', 'radio', 'range', 'reset', 'search', 'submit', 'tel', 'text', 'time', 'url', 'week'],
        'button' => ['submit', 'reset', 'button'],
    );
    const FORM_TAGS_COMMON_ATTR = ['id', 'class', 'accesskey', 'style', 'tabindex'];
    const FORM_TAGS_REQUIRED_ATTR = array(
        'form' => ['name', 'action', 'method'],
        'select' => ['name'],
        'textarea' => ['name', 'cols', 'rows'],
        'button' => ['name', 'value'],
        'option' => ['value', 'text'],
        'input' => ['type', 'name', 'value'],
        'optgroup' => ['label'],
    );
    const FORM_TAGS_SPECIFIC_ATTR = array(
        'form' => ['target', 'enctype', 'autocomplete', 'rel', 'novalidate'],
        'label' => ['for'],
        'select' => ['autocomplete', 'autofocus', 'disabled', 'form', 'multiple', 'required', 'size'],
        'textarea' => ['autocomplete', 'autofocus', 'disabled', 'form', 'maxlength', 'minlength', 'placeholder', 'readonly', 'required'],
        'button' => ['autofocus', 'disabled', 'form', 'formaction', 'formenctype', 'formmethod', 'formtarget', 'formnovalidate'],
        'fieldset' => ['disabled', 'form'],
        'legend' => [],
        'datalist' => [],
        'output' => ['for', 'form'],
        'option' => ['disabled', 'label', 'selected'],
        'optgroup' => ['disabled'],
        'input' => ['disabled', 'required'],
        'checkbox' => ['checked'],
        'color' => [],
        'date' => ['max', 'min', 'step'],
        'datetime-local' => ['max', 'min', 'step'],
        'email' => ['list', 'maxlength', 'minlength', 'multiple', 'pattern', 'placeholder', 'readonly', 'size'],
        'file' => ['accept', 'capture', 'multiple'],
        'hidden' => [],
        'image' => ['alt', 'formaction', 'formenctype', 'formmethod', 'formnovalidate', 'formtarget', 'height', 'src', 'width'],
        'month' => ['list', 'max', 'min', 'readonly', 'step'],
        'number' => ['list', 'max', 'min', 'placeholder', 'readonly', 'step'],
        'password' => ['maxlength', 'minlength', 'pattern', 'placeholder', 'readonly', 'size'],
        'radio' => ['checked', 'required'],
        'range' => ['list', 'max', 'min', 'step'],
        'reset' => [],
        'search' => ['list', 'maxlength', 'minlength', 'pattern', 'placeholder', 'readonly', 'size', 'spellcheck'],
        'submit' => ['formaction', 'formenctype', 'formmethod', 'formnovalidate', 'formtarget'],
        'tel' => ['list', 'maxlength', 'minlength', 'pattern', 'placeholder', 'readonly', 'size'],
        'text' => ['autofocus', 'list', 'maxlength', 'minlength', 'pattern', 'placeholder', 'readonly', 'size', 'spellcheck'],
        'time' => ['list', 'max', 'min', 'readonly', 'step'],
        'url' => ['list', 'maxlength', 'minlength', 'pattern', 'placeholder', 'readonly', 'size', 'spellcheck'],
        'week' => ['max', 'min', 'readonly', 'step']
    );
    const FORM_TAGS_FORBIDDEN_ATTR = []; // to be implemented in the future
    // fieldsets closing is treated in render() method
    const FORM_TAGS_CLOSING = ['label', 'select', 'textarea', 'legend', 'option', 'optgroup'];
    const FORM_BOOLEAN_ATTR = ['required', 'autofocus', 'multiple', 'checked'];

    /**
     *
     * @param array $form Asociative array where the keys are the form tag
     * attributes and the values the attribute's value
     * @throws Exception
     *
     * Attributes name, action and method are mandatory and cannot be empty.
     * The rest of the attributes, if set must have a valid value.
     *
     * Usage:
     *
     *  $form = new Form(['name'=>'myform', 'action'=>'action.php', 'method'=>'post']);
     *
     * Result:
     *
     *  <form name='myform' action='action.php' method='post'>
     *
     */
    public function __construct(array $form_attributes)
    {
        $output = "";

        $this->check_attributes("form", $form_attributes);

        $output = '<form';

        foreach ($form_attributes as $attribute => $value) {
            $output .= " $attribute";
            if (!in_array($attribute, self::FORM_BOOLEAN_ATTR)) {
                $output .= "=\"$value\"";
            }
        }

        $output .= '>';

        $this->output[] = $output;
    }

    public function add_tag(string $element): bool
    {
        $this->check_unset_select();

        // as fieldsets are treated differently and when they have no
        // attributes set needs the trailing '>'
        if (
            ($this->tag == "fieldset") and
            (strpos($this->tag_build, -1) != ">")
        ) {
            $this->output[] = $this->tag_build . ">";
        }

        $this->tag = "";
        $this->tag_build = "";
        $this->select_options_required = false;
        $this->select_options_set = false;

        $this->check_element($element);

        $this->tag = $element;

        $this->tag_build = "<" . $element;

        if ($element == "select") {
            $this->select_options_required = true;
        }
        return true;
    }

    public function add_tag_attributes(array $attributes): bool
    {
        $this->check_attributes($this->tag, $attributes);

        if (array_key_exists($this->tag, self::FORM_TAGS_TYPES)) {
            $this->check_element_type($attributes["type"]);
        }

        foreach ($attributes as $attribute => $value) {
            if ($attribute != "text") {
                $this->tag_build .= " $attribute";

                if (!in_array($attribute, self::FORM_BOOLEAN_ATTR)) {
                    $this->tag_build .= "=\"$value\"";
                }
            }
        }

        $this->tag_build .= ">";

        if (in_array($this->tag, ["label", "textarea", "legend"])) {
            if (array_key_exists("text", $attributes)) {
                $this->tag_build .= $attributes["text"] ?? "";
            }
            $this->tag_build .= "</" . $this->tag . ">";
        }

        if ($this->tag != "select") {
            $this->output[] = $this->tag_build;
        }

        return true;
    }

    public function add_tag_options(array $options, array $selected = []): bool
    {
        $optgroup = false;

        if ($this->tag != "select") {
            throw new Exception("Options are only valid for select tag");
        }

        if (count($options) < 1) {
            throw new Exception("select tag must have at least one option");
        }

        // check if optgroup
        if (count($options) != count($options, COUNT_RECURSIVE)) {
            $optgroup = true;
        }

        foreach ($options as $value => $text) {
            if ($optgroup) {
                $this->tag_build .= "\n<optgroup label=\"$value\">";
                foreach ($text as $opt_value => $opt_text) {
                    $this->tag_build .= "\n<option value=\"$opt_value\"";

                    if (in_array($opt_value, $selected)) {
                        $this->tag_build .= " selected";
                    }

                    $this->tag_build .= ">$opt_text</option>";
                }
                $this->tag_build .= "\n</optgroup>"; // this is not mandatory
            } else {
                $this->tag_build .= "\n<option value=\"$value\"";

                if (in_array($value, $selected)) {
                    $this->tag_build .= " selected";
                }

                $this->tag_build .= ">$text</option>";
            }
        }

        $this->tag_build .= "\n</select>";

        $this->select_options_set = true;
        $this->output[] = $this->tag_build;
        unset($this->tag_build);
        return true;
    }

    public function render(): string
    {
        $open_fieldsets = 0;
        $output = "";

        $this->check_unset_select();

        foreach ($this->output as $element) {
            if (strpos($element, "fieldset") !== false) {
                if ($open_fieldsets > 0) {
                    $output .= "\n</fieldset>";
                    $open_fieldsets--;
                }
                $open_fieldsets++;
            }
            $output .= "\n$element";
        }

        while ($open_fieldsets != 0) {
            $output .= "\n</fieldset>";
            $open_fieldsets--;
        }

        $output .= "\n</form>";
        unset($this->output); // prevents double rendering
        return $output;
    }

    /**
     * Checks if an HTML form tag is valid
     * @param string $declared_element
     * @throws Exception
     * @return boolean
     *
     * As said, PHP 7.0.33 does not allow void return type
     *
     */
    private function check_element (string $declared_element): bool
    {
        if (strlen($declared_element) < 1) {
            throw new Exception("Form tag/element must be a non empty string");
        }

        if (in_array($declared_element, self::FORM_TAGS)) {
            return true;
        }

        throw new Exception("'$declared_element': illegal form (or type of) element");
    }

    /**
     * Checks if input or button has an allowed type
     * @param string $type
     * @throws Exception
     * @return bool
     *
     * As said, PHP 7.0.33 does not allow void return type
     */
    private function check_element_type(string $type): bool
    {
        if (strlen($type) == 0) {
            throw new Exception($this->tag . " type cannot be an empty string");
        }

        foreach (self::FORM_TAGS_TYPES as $element => $types) {
            if (
                ($this->tag == $element) and
                in_array($type, $types)
            ) {
                return true;
            }
        }

        throw new Exception("'$type' type for '" . $this->tag . "' is not valid");
    }

    /**
     * Checks attributes
     *
     * @param string $element
     * @param array $attributes
     * @throws Exception
     * @return bool
     *
     * This method checks for mandatory attributes to be present. Also checks
     * for non mandatory and common attributes and ignores the rest.
     *
     * As said, PHP 7.0.33 does not allow void return type
     */
    private function check_attributes (string $element, array $attributes): bool
    {

        foreach (self::FORM_TAGS_REQUIRED_ATTR[$element] ?? [] as $attribute) {

            if (!array_key_exists($attribute, $attributes)) {
                throw new Exception("Attribute '$attribute' for tag '$element' is mandatory");
            }

            // if attribute is not value or text, and it's not a boolean
            // attribute and its length is 0... not valid
            if (
                ($attribute != "value") and
                ($attribute != "text") and
                (!in_array($attribute, self::FORM_BOOLEAN_ATTR)) and
                (strlen($attributes[$attribute]) == 0)
            ) {
                throw new Exception("Attribute '$attribute' value cannot be an empty string");
            }
        }

        return true;
    }

    /**
     * Check's if select tag options where set
     * @throws Exception
     * @return bool
     *
     * This function is used in add_tag() and render() methods.
     *
     * As said, PHP 7.0.33 does not allow void return type
     */
    private function check_unset_select(): bool
    {
        // check if previous tag was "select" and if options were set
        if (
            ($this->tag == "select") and
            !$this->select_options_set
        ) {
            throw new Exception("Options for select are mandatory");
        }

        return true;
    }

}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ I strongly recommend that you forget about your gut feelings and forget that and and or are keywords in php. Beside all the bad implications mentioned by others, those keywords are used by literally no-one in the community. Your code which uses them will always stand out of line and look uncommon to other devs. Also every code of others that you'll ever read will be written using && and || and you better be friends with them anyway. Also don't forget they are very common in other languages as well and so it's a common ground beyond just php devs. \$\endgroup\$
    – slepic
    Commented May 4, 2023 at 3:45

1 Answer 1

2
\$\begingroup\$

Review

Related to the code itself, the approach on array parameters, efficiency and ease of use, are there any suggestions?

The code has come a long way since the first version posted.

At first glance it doesn't seem overly inefficient. The calls to check* methods typically come early in methods before the main processing (e.g. looping over attributes).

If there is a goal of optimizing for speed then one could considering using simple for loops instead of foreach loops. I don't expect the speed gains to be much but when making a library it is something to consider.

Suggestions

Consider Supported PHP Versions

I am using PHP 7.0.33

If you have control over the PHP version then it would be wise to update it, or if not then it would be wise to request it be updated. At the time of writing there is Active support for version 8.1 and 8.2, Security fixes only for 8.0 and End of life support for 7.2 and earlier. Hence versions like 7.0.33 "may be exposed to unpatched security vulnerabilities." Additionally, PHP 8 has improved performance and numerous other features.

screenshot of PHP Supported versions page

Utilize different string delineators

In the Form constructor there is a loop over the attributes. Within the loop is this conditional block:

if (!in_array($attribute, self::FORM_BOOLEAN_ATTR)) {
    $output .= "=\"$value\"";
}

The HTML specification allows for multiple ways to specify attribute values, including Single-quoted attribute value syntax or Double-quoted attribute value syntax so single quotes could be used instead, which requires no escape characters:

if (!in_array($attribute, self::FORM_BOOLEAN_ATTR)) {
    $output .= "='$value'";
}

Use strict type comparisons where possible

It is a good habit to use strict equality operators whenever possible. It may be faster since no type casting would be required. There are some strict type comparisons already - for example, in the render method:

foreach ($this->output as $element) {
    if (strpos($element, "fieldset") !== false) {

In the add_tag() method there are a couple loose equality checks:

  if (
      ($this->tag == "fieldset") and
      (strpos($this->tag_build, -1) != ">")
  ) {
      $this->output[] = $this->tag_build . ">";
  }

and towards the end:

   if ($element == "select") {

The property $this->tag is a string and parameter $element has a type declared: string, so strict equality can be used in both places.

Re-consider operators given precedence

In mickmackusa's review of the previous code the following was mentioned:

  1. I personally never use and or or in PHP code. This helps to avoid precedence issues and assures code consistency.

PHP has two variations for logical AND i.e. and and && (as well as for logical OR: or and ||). Note that and has lower precedence than many other operators including assignment - =. The code above still uses and with conditions enclosed in parentheses. It would be a good habit to use && instead for cases where parentheses may not be present. As this StackOverflow answer illustrates it can lead to unintended consequences in some scenarios.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks for your answer. I am aware of PHP's version support. Unfortunately this is what I have. As for string delineators I was going around that. As for strict comparisons, what could be the issue when using $this->tag == "fieldset" vs $this->tag === "fieldset"? And regarding operators precedence I find it clearer to read using and and or and using parentheses to establish precedence and use === when comparing against bool values, is it that wrong? \$\endgroup\$
    – julio
    Commented May 3, 2023 at 19:32
  • 1
    \$\begingroup\$ There is no requirement to use strict comparisons, but It is a good habit to use strict equality operators. Note that with PHP 8 0 == "foo" now returns false whereas with PHP 7 and earlier it returnstrue (demo). It isn't wrong to use and - just something to be aware of in case you end up needing to assign a value like $truthiness = $this_one and $that; where $this_one = true; and $that = false; \$\endgroup\$ Commented May 3, 2023 at 23:20
  • \$\begingroup\$ I'm taking into consideration what you said and linked and I'm already updating the code. Related to the code itself, the approach on array parameters, efficiency and ease of use, are there any suggestions? \$\endgroup\$
    – julio
    Commented May 4, 2023 at 12:02
  • \$\begingroup\$ @julio see updated answer which addresses that added question. \$\endgroup\$ Commented May 8, 2023 at 22:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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