0

I have a form with a <select> tag. The select has 3 options with values: Book, DVD, and Table. I want to load classes BookType, DVDType and TableTable respectively which each option is selected. The only function of these classes is to change display style of different divs from none to block. I know i can easily accomplish this using conditionals but i was specifically required not to, so i want to use abstract classes instead. I have created an abstract ProductType :

abstract class ProductType {
    
    abstract public function displayInputs();
    
}

I also created the 3 other classes that extends the ProductType like so;

BookType.php

class BookType extends ProductType {
        
    public function displayInputs(){
         
       <script language="javascript">

            document.getElementById("book-div").style.display = "block";
        
        </script>
    

    }
}

DVDType.php

class DVDType extends ProductType {
   
    public function displayInputs(){
        <script language="javascript">

            document.getElementById("dvd-div").style.display = "block";
        
        </script>

    }
}

TableType.php

class TableType extends ProductType {
        
    public function displayInputs(){
       <script language="javascript">

            document.getElementById("table-div").style.display = "block";
        
        </script>

    }
}

Now the issue is i don't know how to get the selected option and call on the appropriate class so that the different divs would show up. Please i would really appreciate any help i can get.

NB: I'm using MVC model.

Thank you in advance.

Claire
  • 19
  • 2
  • _"The only function of these classes is to change display style of different divs from none to block."_ - then I can hardly see how this would justify those classes in the first place. Why does the ProductType class hot have a type property then, based on which the displayInputs method produces slightly different output? – CBroe Jun 03 '22 at 09:13
  • I am just learning PHP. I do no know much. I did what i understood from some tutorials i watched. Let's say i add a property `$getDiv` to the ProductType class, how do i get the displayInputs method to produce slightly different result based on which option is selected? – Claire Jun 03 '22 at 15:59
  • @dakis Thank you so much for the help. I have been able to solve the problem. – Claire Jun 07 '22 at 21:16
  • You are welcome. That's nice to hear. But could you please tell us how you solved it? For that, It would be best to write an answer to your question yourself. – PajuranCodes Jun 08 '22 at 10:40
  • Please Open this Link I think You can find exact Answer; https://stackoverflow.com/questions/48377525/replace-conditional-with-polymorphism – Vipin Singh Jun 10 '22 at 12:13
  • First, your `displayInputs` methods do not contain correct PHP code; you would need to *echo* in some fashion the JavaScript, for example by surrounding the tags with `?>` and ` – Booboo Jun 11 '22 at 12:27

4 Answers4

1

It's better to leave the "View" logic to the template engine, for example, twig

If this is not possible, use form package, such as symfony/form

If two options above are not possible, write own "form builder" by Composite design pattern

cetver
  • 11,279
  • 5
  • 36
  • 56
1

It looks to me you're creating the different classes only for the display parameter of the DOMElement#id. Whether or not that makes sense (lets keep preference out of the answer), what you have is a 1:1 relationship between the DOMElement#id and each concrete ProductType.

This relationship is abstract, therefore you can use the abstract base-class already to encode the name of the type with the class:

abstract class ProductType
{
    private function typeName(): string
    {
        $pathname = $this::class;
        $basename = ltrim(substr($pathname, (int)strrchr($pathname, '\\')), '\\');
        if (!str_ends_with($basename, 'Type')) {
            throw new BadMethodCallException('Hierarchy error');
        }

        return strtolower(substr($basename, 0, -4));
    }

    final public function displayInputs() : ?string
    {
        try {
            $idString = json_encode(sprintf('%s-div', $this->typeName()), JSON_THROW_ON_ERROR);
        } catch (BadMethodCallException|JsonException) {
            return null;
        }

        return <<<HTML
            <script>document.getElementById($idString).style.display = "block";</script>

            HTML;
    }
}

This example also already finally implements the abstract ProductType::displayInputs() method as all classes that are a ProductType use the same code (template):

class BookType extends ProductType {}
class DVDType extends ProductType {}
class TableType extends ProductType {}

You can then use that 1:1 relationship to map the value of the <select> to a concrete ProductType.

<form>
  <select name="ProductType">
    <option>Book</option>
    <option>DVD</option>
    <option>Table</option>
  </select>
  <button>
</form>

The HTML with its tags already represents the actual hierarchy well. All you need is to have your ProductType to also provide the option:

abstract class ProductType 
{
    ...

    public static function option(string $ProductType): self
    {
        try {
            return new("{$ProductType}Type");
        } catch (Throwable) {
            throw new InvalidArgumentException("Name error: '$ProductType'");
        }
    }
    ...

This is already functional:

$_GET['ProductType'] = 'DVD';
var_dump(ProductType::option(...$_GET));
object(DVDType)#1 (0) {
}

And there are no more conditionals regarding the sub-types.

Now to turn this static class hierarchy into something polymorphic, you have to give it an interface:

interface Product
{} # intentionally left empty

and make the concrete classes implement that interface:

abstract class ProductType implements Product
{
    ...

and make the abstract creation method adhere to the interface, not the abstract base-type:

    ...
    public static function option(string $ProductType): Product # was: self

As this is a bare-minimum implementation of polymorphism by your question, the interface can remain empty up to this point as there is a single type only, the product, and all implementations that exist are actually 100% from that of a single template as well, and therefore this could be pretty close to a single implementation only as well.

There is not much more use, but having the interface for the future (it enables polymorphism).

A real-life example would perhaps start with the interfaces and create systems delegating the work across the different layers (MVC and all the different parts of a software).

This is normally not necessary with PHP (as it does not need to be compiled), but if you'd like (or must), I'd start with the interfaces, loader- and unit-tests first as PHP code isn't compiled and needs to be run for verification (does the whole code across different files load and compute?).

There is another benefit with early testing: If you start to create the design without tests early on, it is easy to often walk into a wrong direction until noticed, so early testing reveals interface and methods more easily.

hakre
  • 193,403
  • 52
  • 435
  • 836
  • 1
    Thank you for taking your time to explain all these! I have read it over and over again and it's beginning to make more sense to me so i decided to implement it. I got this error `Cannot use ::class with dynamic class name` on this line `$pathname = $this::class;` – Claire Jun 10 '22 at 09:28
  • @Claire: That is PHP >= 8.0 syntax, you're likely using an older version. Replace `$this::class` with `get_class($this)`, it evaluates to the same string, the class-name of `$this`. See also https://php.net/get_class and the [5th bullet point in PHP 8.0 Other New Features](https://www.php.net/manual/en/migration80.new-features.php#migration80.new-features.core.others). And mind that currently [PHP 8.0 is the _lowest_ actively supported version](https://www.php.net/supported-versions.php), I'd suggest to consider using the current one for development (PHP 8.1). – hakre Jun 10 '22 at 09:53
0

You shouldn't really do it that way, because if you do submit a form page refreshes either way and it's easier to directly change the element instead of adding a script to the page.

If you really want to do it this way then

  1. use $_POST to get value of chosen option from submitted form
  2. depending on the option pick right class to instantiate
  3. call the method displayInputs on object created in step 2

You will have to fix the displayInputs as it doesn't output the script, you should change values of that function to strings and return them, after that echo what you received.

public function displayInputs(){
   return <<<'EOT'
            <script language="javascript">

            document.getElementById("table-div").style.display = "block";
    
            </script>
EOT;
}

This uses nowdoc syntax, in this case it's the best way to output a string spanning multiple lines. There is also heredoc but that does parsing (which we don't need).

If you look closely you will see that step 2 requires use of conditionals and you can't really escape that. You can hide it in a factory or use a dependency injection container, but still there is a conditional somewhere. You can use match expression but that is a conditional too.

Well, there is a specific way to escape conditionals, you create an array keyed with options and with values of functions creating right class object. I'm not sure this is what they had in mind when you were asked to not use conditionals, but I don't see any other kind of safe and maintainable way.

$options = [
    'book' => fn()=>return new BookType(),
    'dvd' => fn()=>return new DVDType(),
    'table' => fn()=> return new TableType()
];
$instance = $options[$_POST['option']]();
echo $instance->displayInputs();

The fn() notation is called arrow functions, this creates functions for calling later (also named closures or lambdas). Later you pick a function and call it (look at parentheses at the end of $instance line). This gives you correct object and all that is left is to call the displayInputs method. This of course assumes what values your form sends, it may not work without changes.

  • This gave me a pretty good idea on how to go about the problem. However, as you stated `This of course assumes what values your form sends, it may not work without changes.` because i want to make the changes without sending the form, i decided to use JavaScript instead of PHP. I searched online and found (https://codepen.io/jenovs/pen/bGYqPLN). It worked as expected. Thank you so much – Claire Jun 07 '22 at 21:10
0

In my opinion, that's a lot of overkill. If you are using PHP, why output all three DIVs when you just want to display one of them? Should they be able to be displayed later with some additional JavaScript code?

Ok, if you really need to do that, why using JavaScript to do CSS stuff when it's only used to display certain sections (or not).

If you have access to the <HEAD> section (and that's not too complicated due to your framework), I would add something like this:

if (in_array($_POST['option'] ?? '', ['book', 'dvd', 'table'])) {
    echo '<link rel="stylesheet" href="/styles/'.$_POST['option'].'.css">';
}

Then have these three CSS-files:

/* book.css */
book-div  { display: block; }
dvd-div   { display: none; }
table-div { display: none; }

/* dvd.css */
book-div  { display: none; }
dvd-div   { display: block; }
table-div { display: none; }

/* table.css */
book-div  { display: none; }
dvd-div   { display: none; }
table-div { display: block; }

In this way you just use the exact CSS file you need to display one of the three DIVs. This doesn't hurt the option to display one of the others later by JS.

Aranxo
  • 677
  • 4
  • 15