Page 1 of 1

Zend Framework MVC Issues

Posted: Wed Jul 05, 2006 7:53 am
by Ollie Saunders
I've been rewriting a medium sized, very procedurial PHP4/MySQL web application in glorious OO PHP 5 Zend Framework and I've been learning a lot in the process. Some of you might think I'm a bit crazy using a technology a new as the ZF in an application that is going to be used in production but I have faith in my abilities as a tester and the fact that I can circumvent/modify the ZF wherever necessary.

Anyway my problem is, I've got a view script that looks like this:

Code: Select all

<?php
require 'administrationHeader.php';
?>
<ul class="buttons">
    <li><a href="/Site/Create/">New Site&hellip;</a></li>
    <li><a href="#" onclick="return tfb.confirm()">Delete Selected</a></li>
</ul>
<br style="clear:left" />
<form method="post" action=".">
<?php
if(!is_null($this->deletedRows)) {
    echo '<p>';
    if($this->deletedRows == 0) {
        echo 'No rows deleted';
    } else if ($this->deletedRows == 1) {
        echo 'A single row was deleted';
    } else {
        echo $this->_englishNumbers($this->deletedRows) . ' were rows deleted';
    }
    echo '</p>';
}
if($this->sites->num_rows) {
    ?>
<table class="lst">
<tr>
    <td class="chkbox" style="visibility:hidden"></td>
    <th class="v_actions" scope="col" colspan="5">Actions</th>
    <th class="v_title" scope="col">Title</th>
    <th class="v_client" scope="col">Client</th>
    <th class="v_status" scope="col">Online</th>
</tr>
    <?php

    while($site = $this->sites->fetch_object()) {
        $html = $this->_bulkSubEscape(clone $site, $this->detailedLimit);
        $html->siteId = (int)$site->siteId;
        $html->longHqVideoUrl = $this->_escape($site->hqVideoUrl);
        $html->longLqVideoUrl = $this->_escape($site->lqVideoUrl);

        $html->isOnlineStr         = var_export((bool)$site->isOnline, true);
        $html->useQuestionnaire = var_export((bool)$site->isOnline, true);
        $html->useVideo         = var_export((bool)$site->isOnline, true);

        $html->ucFirstIsOnlineStr = ucfirst($html->isOnlineStr);
        if (is_null($site->hqVideoUrl)) {
            $html->hqVideoUrl = $this->undefinedMessage;
        }
        if (is_null($site->lqVideoUrl)) {
            $html->lqVideoUrl = $this->undefinedMessage;
        }
        $html->period = $this->_getPeriod($html->wentOnline, $html->wentOffline);
    
        echo <<< END
<tr>
    <td class="chkbox">
        <input type="checkbox" onclick="tfb.selectRecord(this)" name="chkRecord_{$html->siteId}"
        id="chkRecord_{$html->siteId}" value="{$html->siteId}" />
    </td>
    <td class="v_action" title="Modify Site&hellip;">
        <a href="/Site/modify/id/{$html->siteId}">
            <img src="/gfx/ico16/mod.png" alt="Modify Site&hellip;" />
        </a>
    </td>
    <td class="v_action" title="Modify Questions&hellip;">
        <a href="/Question/list/id/{$html->siteId}">
            <img src="/gfx/ico16/modq.png" alt="Modify Questions&hellip;" />
        </a>
    </td>
    <td class="v_action" title="Clone">
        <a href="/Site/clone/id/{$html->siteId}" onclick="return confirm('Sure?')">
            <img src="/gfx/ico16/cln.png" alt="Clone" />
        </a>
    </td>
    <td class="v_action" title="Preview&hellip;">
        <a href="/Site/preview/id/{$html->siteId}">
            <img src="/gfx/ico16/pre.png" alt="Preview&hellip;" />
        </a>
    </td>
END;
        if($html->isOnline) {
            echo '<td class="v_action" title="View Online&hellip;">';
            echo '<a href="/Pub/id/' . $html->siteId . '"><img src="gfx/ico16/go.png" alt="Disabled" /></a>';
            echo '</td>';
        } else echo '<td>&nbsp;</td>';
        echo <<< END
    <td class="v_title">
        <a name="site{$html->siteId}" href="/Site/list/id/{$html->siteId}#site{$html->siteId}">
            {$html->title}
        </a>
    </td>
    <td class="v_client">
        <a href="/Client/list/id/{$html->clientId}">{$html->clientName}</a>
    </td>
    <td class="v_status">
        <img src="gfx/ico16/{$html->isOnlineStr}.png" alt="{$html->isOnlineStr}" />
    </td>
</tr>
END;
    }
    echo '</table></form><br />';
    echo $this->_pageNav($this->page, 'Site');
} else echo '</form><p>No sites in database</p>';
require 'administrationFooter.php';
The problem is this isn't nearly structured enough for me. It seems too similar to the logic, markup soup I was using in the previous version of this application. So I've been pondering to myself about how to make this more structured and also how structured should this be?

The best idea I have come up with is to extend Zend_View into a global class called TillingView (Tilling being the prefix for all my classes in this project) and then extend that to TillingViewList for list pages (pages where output is displayed, you can delete rows and there is page spanning functionality) and extend that to TillingViewListSite which would be specific to this particular view script.

The aim of doing all this would be to then have enough methods so that the view script itself would almost have no markup in it at all just calls to helper methods that either format data or output markup structure with subsitutations. This would of course also have the advantage that all the list page I create (of which there are 4, site, report, answer set and question) I have TillingViewList which will be common to them all.

I'm still getting to grips with the idea of MVCs but what do you think of my ideas here?

Posted: Wed Jul 05, 2006 8:50 am
by asgerhallas
I'm sorry, but I'm not sure I fully get it... which methods would you put into your new class? Can I have an example or two?

I'm not sure why you should derive a special view class for a special view, shouldn't the controller be doing any work?

/Asger

Posted: Wed Jul 05, 2006 9:31 am
by Ollie Saunders
I'm not sure why you should derive a special view class for a special view, shouldn't the controller be doing any work?
Yeah the controller does all the business logic. These methods would be for output only.
Can I have an example or two?
Yes you can.

Here are two functions I would put in TillingView

Code: Select all

/**
 * Converts numbers less than 11 to there english word eqivilents, proper
 * grammar styler! XD
 * 
 * @param int $number
 */
public function _englishNumbers($num) 
{
	$english = array('one','two','three','four','five',
					 'six','seven','eight','nine','ten');
	if(isset($english[$num])) return $english[$num];
	else return $num;
}
/**
 * Limit the length of $str and escape. Where $str has to be truncated,
 * append with &hellip;
 *
 * @param string $str
 * @param int $limit
 * @return string
 */
public function _subEscape($str,$limit) 
{
	if(strlen($str) > $limit) {
		return $this->_escape(substr($str,0,$limit)) . '&hellip;';
	} else {
		return $this->_escape($str);
	}
}
The kinds of methods that would be of use throughout the application. While these two:

Code: Select all

/**
 * Produces page span navigation links (next and previous) and infomation
 * from a TillingPageSpan object.
 * 
 * @param TillingPageSpan $page
 * @param string $name name of the entity this page is referring e.g. Site
 * @return string
 */
public function _pageNav(TillingPageSpan $pg, $name) 
{
	
	$out = '<span style="float:right">'; {
		// previous link
		if ($pg->page > 0) {
			$out.= '<a href="/' . $name . '/list/page/' . ($pg->page-1) . '">< Previous</a>';
		} else {
			$out.= '< Previous';
		}
		$out.= '&nbsp; | &nbsp;';
		// next link
		if ($pg->page < $pg->lastPage) {
			$out.= '<a href="/' . $name . '/list/page/' . ($pg->page+1) . '">Next ></a>';
		} else {
			$out.= 'Next >';
		}
	} $out.= '</span>';
	
	// summary information
	$out.= 'Showing ' . strtolower($name) . 's ' . $pg->startRow . ' to ' . $pg->endRow . ' of ' . $pg->rowsInTable;
	return $out;
}
/**
 * Formats the online and offline dates into a string that represents
 * a slice of time
 * 
 * @param null string $wentOnline
 * @param null string $wentOffline
 * @return string
 */
public function _getPeriod($wentOnline, $wentOffline) {
	if (is_null($wentOnline)) {
		return 'Not yet published';
	} else {
		if(is_null($wentOffline)) $wentOffline = 'date';
		return $wentOnline . ' to ' . $wentOffline;
	}
}
would be placed in TillingViewList because I'd only use them on list pages.

Posted: Wed Jul 05, 2006 9:40 am
by asgerhallas
Ahh ok, I see... I think such functionality ought to be in the Zend_View_Helpers_* classes...

http://framework.zend.com/manual/en/zen ... lpers.html

But maybe I still don't understand the question?

/Asger

EDIT: It's a known issue, that the view helpers doesn't apply to the naming convention of the Zend Framework as individual project code should be prefixed Zend_. But then again, it's still alpha...
http://framework.zend.com/issues/browse/ZF-148

Posted: Wed Jul 05, 2006 10:26 am
by Ollie Saunders
yeah but i don't like those because you have to create a whole class for each method and to create a object hierarchy using them would be unnecessarily difficult.

Posted: Wed Jul 05, 2006 12:23 pm
by Yossarian
public function _getPeriod($wentOnline, $wentOffline) {
if (is_null($wentOnline)) {
return 'Not yet published';
} else {
if(is_null($wentOffline)) $wentOffline = 'date';
return $wentOnline . ' to ' . $wentOffline;
}
}
I am sorry but it looks as though you might be getting ahead of yourself using ZF at this stage.

Don't get me wrong now, _getPeriod is a nice thing, and I bet it saves you loads of time.

The fact is though that even if it was a class, or a method in a class, you are using it to write a string, when OOP thinking would probably say it should return 2 values or possibly a single value (the time diff). This tells me you are probably going to have a bad day trying to leverage the power of MVC. (sigh, tell me about it...)

How you use those values, be they timestamps or whatever, is up to another view, or object higher up the chain.

I think you might be able to get away with using them as they are though, simply wrap them all in a class MyHelperClasses and call them statically.
yeah but i don't like those because you have to create a whole class for each method and to create a object hierarchy using them would be unnecessarily difficult.
yeah, but theres the rub, you cant really profit from reusable code until you make more effort to understand OOP.... (me? 18 months after buying my 1st OOP book, still thrashing around... still, gotta make a shilling.)

Posted: Wed Jul 05, 2006 12:29 pm
by Ollie Saunders
_getPeriod isn't supposed to do any maths, it just formats 2 timestamps (which can sometimes be null) for output.

Re: Zend Framework MVC Issues

Posted: Wed Jul 05, 2006 12:45 pm
by Christopher
ole wrote:The best idea I have come up with is to extend Zend_View into a global class called TillingView (Tilling being the prefix for all my classes in this project) and then extend that to TillingViewList for list pages (pages where output is displayed, you can delete rows and there is page spanning functionality) and extend that to TillingViewListSite which would be specific to this particular view script.

The aim of doing all this would be to then have enough methods so that the view script itself would almost have no markup in it at all just calls to helper methods that either format data or output markup structure with subsitutations. This would of course also have the advantage that all the list page I create (of which there are 4, site, report, answer set and question) I have TillingViewList which will be common to them all.

I'm still getting to grips with the idea of MVCs but what do you think of my ideas here?
This is exactly what you should do and how it seems to be intended to work. A number of other programmers have created their own bas View object as well. You might also want to extend the Action Controller to create your own that deals with pages in the manner that you like. That way you can offload a little of the stuff from the View into the Controller as appropriate for you.

Posted: Wed Jul 05, 2006 1:56 pm
by Ollie Saunders
Thank you arborint for your positive response :)

I've come to realise that what I'm asking is:
How can I best identify and implement reusable output?

I was looking at that big block of code I posted at the top and trying to think how I could abstract it. I thought here, there's quite a bit of repetition:

Code: Select all

<td class="v_action" title="Modify Site&hellip;">
		<a href="/Site/modify/id/{$html->siteId}">
			<img src="/gfx/ico16/mod.png" alt="Modify Site&hellip;" />
		</a>
	</td>
	<td class="v_action" title="Modify Questions&hellip;">
		<a href="/Question/list/id/{$html->siteId}">
			<img src="/gfx/ico16/modq.png" alt="Modify Questions&hellip;" />
		</a>
	</td>
	<td class="v_action" title="Clone">
		<a href="/Site/clone/id/{$html->siteId}" onclick="return confirm('Sure?')">
			<img src="/gfx/ico16/cln.png" alt="Clone" />
		</a>
	</td>
	<td class="v_action" title="Preview&hellip;">
		<a href="/Site/preview/id/{$html->siteId}">
			<img src="/gfx/ico16/pre.png" alt="Preview&hellip;" />
		</a>
	</td>
I could make a function that would generate each TD and here I've done it:

Code: Select all

function actionButton($controller, $action, $id, $title, $useHellip, $iconFile) 
{
	$out = '<td class="v_action" title=' . $title;
	if($useHellip) $out .= '&hellip;';
	$out .= '"><a href="' . $controller . '/' . $action . '/id/' . $id . '">';
	$out .= '<img src="/gfx/ico16/' . $iconFile . '.png" alt="' . $title . '" /></a></td>';
	return $out;
}
echo actionButton('Site', 'modify', $html->siteId, 'Modify Site', true, 'mod');
echo actionButton('Question', 'list', $html->siteId, 'Modify Questions', true, 'modq');
echo actionButton('Site', 'clone', $html->siteId, 'Clone', false, 'cln');
echo actionButton('Site', 'preview', $html->siteId, 'Preview', true, 'pre');
What do you think of that as a practice?

I've developed some opinions having done it:

The good
  • It is more structured, the parameters demand that you supply the right data. I can format the input I supply to the function in the function.
  • I could reuse it, in fact I'm likely to need to make quite a few actionButtons in this application so that's good
  • Its debatibly easier to read
  • I could use it in a loop
The bad
  • I've sacrified to my ability to be specific in order to obtain structure. If I want to cater for specifics my function will get very complex very quickly. Say I wanted to add that single onclick event to one of the actionButtons.
  • It took longer to write
So what might combine the good and the bad? Well what about the JavaScript DOM?

Code: Select all

// Not including onclick support directly
function actionButton(sController, sAction, nId, sTitle, bUseHellip, sIconFile) 
{
	var dTd = d.createElement('TD');
	dTd.className = 'v_action';
	dTd.title = sTitle;
	if(bUseHellip) dTd.title += '&hellip;';
	
	var dA = d.createElement('A');
	dA.href = '/' + sController + '/' + sAction + '/id/' + nId;
	
	var dImg = d.createElement('IMG');
	dImg.src = '/gfx/ico16/' + sIconFile + '.png';
	dImg.alt = title;
	
	dA.appendChild(dImg);
	dTd.appendChild(dA);
	return dTd;
}
var dTd = [];
dTd[0] = actionButton('Site', 'modify', html.siteId, 'Modify Site', true, 'mod');
dTd[1] = actionButton('Question', 'list', html.siteId, 'Modify Questions', true, 'modq');
dTd[2] = actionButton('Site', 'clone', html.siteId, 'Clone', false, 'cln');
dTd[3] = actionButton('Site', 'preview', html.siteId, 'Preview', true, 'pre');
// I can make an exception to the rule without altering the logic of the function 
// Here I am able to add onClick to one of the TDs:
dTd[2].firstChild.firstChild.onClick = 'return confirm(\'Sure?\')';
Clearly the JavaScript DOM is an awesome solution to this problem. But I think I've reached my own conclusion that unless I start using a PHP DOM to generate all my output (which I think would be a performance hit too many) I've got to write out all my interface output in a monolithic fashion.
You might also want to extend the Action Controller to create your own that deals with pages in the manner that you like.
Yep, i've already done that.

Posted: Wed Jul 05, 2006 8:16 pm
by Ollie Saunders
Well I went for it, and I managed to turn markup soup (see first post) into beautiful standardized output ahhhh:

Code: Select all

<?php
require 'administrationHeader.php';

echo $this->_beginButtons();
echo $this->_button(false, '/Site/Create/', 'New Site&hellip;');
echo $this->_button(true, 'return tfb.confirm()', 'Delete Selected');
echo $this->_endButtons();

echo $this->_beginForm();
echo $this->_deletedRowsMsg();

if($this->sites->num_rows) {
	?>
<table class="lst">
<tr>
	<td class="chkbox" style="visibility:hidden"></td>
	<th class="v_actions" scope="col" colspan="5">Actions</th>
	<th class="v_title" scope="col">Title</th>
	<th class="v_client" scope="col">Client</th>
	<th class="v_status" scope="col">Online</th>
</tr>
	<?php

	while($site = $this->sites->fetch_object()) {
		$html = $this->_bulkSubEscape(clone $site, $this->detailedLimit);
			
		echo '<tr>';
		echo $this->_checkboxForDelete($html->siteId);
		echo $this->_actionIcon('/Site/modify/id/', $html->siteId, 'Modify Site&hellip;', 'mod.png');
		echo $this->_actionIcon('/Question/list/id/', $html->siteId, 'Modify Questions&hellip;', 'modq.png');
		echo $this->_actionIcon('/Site/clone/id/', $html->siteId, 'Clone', 'cln.png', 'return confirm(\'Sure?\')');
		echo $this->_actionIcon('/Site/preview/id/', $html->siteId, 'Preview&hellip;', 'pre.png');
		
		if($html->isOnline) {
			echo $this->_actionIcon('/Pub/id/', $html->siteId, 'View Online&hellip;', 'go.png');
		} else echo '<td>&nbsp;</td>';
		
		echo '<td class="v_title"><a name="site'. $html->siteId . '" href="/Site/list/id/' . $html->siteId . '#site' . $html->siteId . '">' . $html->title . '</a></td>';
		echo '<td class="v_client"><a href="/Client/list/id/' . $html->clientId . '">' . $html->clientName . '</a></td>';
		
		if($html->isOnline) {
			echo $this->_actionIcon('/Site/unpublish/id/', $html->siteId, 'Unpublish', 'true.png');
		} else {
			echo $this->_actionIcon('/Site/publish/id/', $html->siteId, 'Publish', 'false.png');
		}
		echo '</tr>';
	}
	echo '</table>';
	echo $this->_endForm();
	echo '<br />';
	echo $this->_pageNav($this->page, 'Site');
} else {
	echo $this->_endForm();
	echo '<p>No sites in database</p>';
}
require 'administrationFooter.php';
Now doesn't that look nice :D
And now I have a bunch of method that I can make use of in all my other list pages.

Posted: Thu Jul 06, 2006 6:34 am
by asgerhallas
Oh yeah... It actually is beautiful :-)

Now I just have one question... if extending the view class is the best solution, what is the view helpers for then?

/Asger

Posted: Fri Jul 07, 2006 1:56 pm
by Ollie Saunders
if extending the view class is the best solution, what is the view helpers for then?
Well you have to wonder don't you.
I don't quite know what Zend was thinking. I expect they have a very good reason but its not obvious to me.

Posted: Fri Jul 07, 2006 3:33 pm
by John Cartwright
ole wrote:
if extending the view class is the best solution, what is the view helpers for then?
Well you have to wonder don't you.
I don't quite know what Zend was thinking. I expect they have a very good reason but its not obvious to me.
Last time I used ZF, I extended the Zend View to basically wrap the functionality of a template engine such as smarty instead of the default usage.