tags:

views:

61

answers:

2

Consider the following from CodeIgniter. If I am doing this wrong, then please tell me:

<html>
<head>
    <title><?=$title ?></title>
</head>
<body>
    <h1><?=$heading; ?></h1>


     <?php foreach ($query->result() as $row):?>
    <p>
     <?=$row->LastName . ', ' . $row->FirstName . ' ' . $row->MiddleName?>
     </br>
     <?=$row->myField?>
     <?php if ($row->myField == ''):?>
     no data found
     <?php endif;?>
     some text
     <?=$row->anotherField ?>
     </br>
     <?php if ($row->purchasedate !='') {echo 'Purchase Date ' . mdate('%m' . '/' . '%d' .  '/' . '%Y', strtotime($row-> purchasedate));}?>

    </p>  
    <p>
      <label for="ProductNumber">Product</label>
      <input type="text" name="Product_number" id="ProductNumber" value = 
    <?=$row->ProductNumber?>
     />
    </br>
    <?php if ($row->Purchased == '-1'): ?>
    <h3>Bought</h3>
    <?php endif;?></br>
    <?php if ($row->Sold == '-1'): ?>
    <h3>Sold</h3>
    <?php endif;?></br>
    </p>

      <?php endforeach;?>

</body>
</html>

I know some of it will not make sense. I am experimenting and also changed some of the names of fields.

My question is: Is this too much intermixing of code into my view? It seems this occurs with most templating but that there is some vague line that just feels wrong when you cross it and you say, "That's too much logic in my view." I don't know that line. Is this example crossing it? A foreach, if thens, echo, strtotime, the ?php tags, etc.

Is it just crossing the "line" when I put db access logic and start emitting all my html tags from print or response.write statements all on one big page behind the scenes on the server?

+3  A: 

There's better checks you can do instead of shoving that logic in the view. For example you're doing things like if($row->Purchased == '-1') when in fact it would be better to have if($row->isPurchase), that way determining the validity and control flow is placed further onto the state of your model. If you followed this logic throughout your code example you'll see it actually turns into something much simpler.

Kezzer
I'm not sure what you mean. Maybe it's because I haven't worked with the model yet, but what is "isPurchase"?
johnny
You're determining if it's a purchase by checking how many are purchased I assume? Well this kind of logic shouldn't be done in the view, you should simply have a flag indicating whether it's a purchase or not which is what "isPurchase", it tells you IF it is a purchase.
Kezzer
A: 

MVC Question: How do I know when I’m putting too much logic into my view?

IMHO

If you're logic is doing anything other than making presentational decisions. In the example all you're if statements are doing something graphical. So I think there is nothing wrong with that code.

Using all those php tags can make a view look like a mess. But thats just php. Thats why I use smarty, it makes it a bit more readable. Yeah I know php is a template language but its just my opinion.

MrHus
@MrHus: Clarification: When you say "And that is exactly what you are doing in your example." do you mean: his logic *is* doing something other than making presentational decisions or it *isn't*?
Grant Wagner
@Grant: I'm glad you asked that because I too was confused. I was happy that I might have done something right and then sad again.
johnny
@MrHus: you're correct, checking against '-1' values isn't presentational decisions, it's business logic that's existing in the view.
Kezzer