Refactor: Which code is okay to put in the View?

[dennis-fedco]

Say I have a view page that prints description lines for various product numbers, like 1, 2, 3, 4, 5.  For each product the description lines are the same:

$html .= "
    Line X<br>
    Line Y<br>
    Line Z<br>";

Now, customer says:  Remove (Do not show) Line Y for products 2, 4, 5.  How best implement this change?

 

I am thinking of doing this, right in the view:

$modelNumber = extractModelNumber($productName);  //model number is not directly available
if (!in_array($modelNumber, array(2, 4, 5))       //hide line as requested
{
    $html .= "
        Line X<br>
        Line Y<br>
        Line Z<br>";
}

Ok, not Ok?  I think it will be fair to say that this change is considered to be "business logic", and hence it does not belong in the view.  What then may you suggest I do?

[gizmola]

In the model data structure you would want to add an element to those specific data items that indicates it should not show lineY.

 

Then your view structure simply needs to do a quick boolean check, and only output that line in the view when needed.

 

Also your view should not be creating variables like that and outputting them.  Use a mixture of html and the alternate php syntax as much as possible.  The point of a view is that it localizes the specific required look and feel of the model data.  I'd expect your view to be more like:

 

 

<ul>
   <?php foreach ($products as $product): ?>
      <li>
           <?= $product['x'] ?> <br>
           <?= $product['okY'] ? $product['y'] : ''; ?>
           <?= $product['z'] ?>
      </li> 
   <?php endforeach; ?>
</ul>

[requinix]

Disclaimer: lots of stuff about MVC is debatable. The below is my opinion on how it should all work.

 

 

Determine the reason for the change. Line Y must be something significant for it to be deliberately removed from the page. What is it? Why is it being removed and not X or Z?

 

Then you need to consider what roles and responsibilities are given to the model, controller, and view. Since you (now) know the reason for the change you can decide which of those three should be in charge of any relevant decisions.

- The model is in charge of the properties and behaviors inherent to the thing (ie, product) itself. For example, products inherently have a name so that should be handled in the model.

- The controller is in charge of ferrying data and operations between the view (what the user interacts with) and the model (where the data is stored). If someone wanted to change a product's name then you would do the "if they're changing the name then make the product's name change" logic in the controller, keeping in mind that the name itself should still be handled by the model. (That is the result would be the controller telling the product model to change its name to some new value, or simpler that the name has changed to a new value and the model should commit the change.)

- The view is in charge of outputting data and making sure the controller can get inputted data as well (ie, setting up

s and the like). It knows about the HTML, it knows about the structure of the page, and it knows about how the data should be placed on the page.

 

Without actually knowing your system I'm going to assume that the three line things were supposed to be tidbits of information that applied to all products, except now they apply to all but those three you're dealing with.

 

Information itself belongs in the model because that is what is responsible for products' properties, and the line things are properties of the product. All (almost all) products, perhaps, but still of the products themselves. So instead you should be accessing them, and hopefully storing them too, with the model. Next the controller gets the product model and, surely among other things, grabs the line things from the model so that they can be sent to and displayed in the view. All it sends is the three "Line X", "Line Y" (if desired), and "Line Z" bits - no HTML markup. The controller doesn't even care what the actual line items are because what they are has no bearing on how the controller operates. When the view takes over it receives those three (or two) line things and decides to output them as HTML using

s to separate each. Again, the view doesn't care what the line items are because what they are doesn't matter for how the view displays them.

 

Product model

class Products extends Model {

	public function getLineThings() {
		/* exactly what you do here depends where these line things come from.
		   while they really shouldn't be, I'm just going to hardcode them here. */

		// the particular products #2, #3, and #5 have X and Z
		if ($this->id == 2 || $this->id == 3 || $this->id == 5) {
			return array("Line X", "Line Z");
		}
		// otherwise all other products have X, Y, and Z
		else {
			return array("Line X", "Line Y", "Line Z");
		}
	}

}

Page controller

class WhateverPageThisIsController extends Controller {

	public function execute() {
		/* (pretending this is where the controller executes itself) */

		$product = Products::getParticularProduct($some_inputted_value_or_something);

		$view = $this->getViewDataBag();
		$view->linethings = $product->getLineThings();

		/* and now the relevant view gets executed using the data I just put in $view */
	}

}

View

<?php

$data = $this->getViewDataBag(); // the $view from the controller

// ...

// now output the line things
$linethings = $data->linethings;
// the data is just an array - it needs to be turned into HTML
$html .= implode("<br>", $linethings); // Line X<br>Line Y<br>Line Z

// ...

[dennis-fedco]

Thanks!  Right now my "View" is part of legacy code.  As such, adding complex models/controllers/views is out of the question.  Adding those structures ad-hoc into legacy code now will only complicate things, at least in the short term. 

 

 

Overall I agree - yes, there is a need to find out why this change is being made.  This can be a thing that does not exist on certain models, and that's that.