public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alexey Borzov <[email protected]>
To: Gavin M. Roy <[email protected]>
Cc: Joshua D. Drake <[email protected]>
Cc: Josh Berkus <[email protected]>
Cc: [email protected]
Subject: Re: [pgsql-www] PostgreSQL.org Design Proposal
Date: Wed, 03 Nov 2004 14:25:28 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <E7F85A1B5FF8D44C8A1AF6885BC9A0E4306E8F@ratbert.vale-housing.co.uk>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi,

Gavin M. Roy wrote:
>>> 1) HTML generated as PHP strings;
>>>   It is not necessary to use some "template engine" but code 
>>> generating data structures for output should be separate from code 
>>> outputting this data which can be e.g. HTML file with minimal PHP. 
>>> Changing the design done in the following way is *extremely* difficult:
>>>
>>>       $ret .= "\n<form name='edit' action='$this->action' 
>>> method='$this->method'>\n";
>>>       $ret .= "<blockquote><table class='form' width='98%' 
>>> border='0'>\n";
>>>       foreach($this->fields as $field)
>>>       {
>>>         $ret .= "  <tr valign='top'>\n";
>>>         $ret .= "    <td class=\"formitem\"";
>>
> This is the form object which is for backend admin form generation.  It 
> generally has one purpose which is to make two column forms via 
> objects.  While I'm not the author of this set of objects, I'd be happy 
> to implement suggestions.  

You mean people using your framework will be unable to use this form-generation 
class in the frontend of the website? Will they need to code all forms by hand? 
'Cause if you expect them to use it in frontend, they'll definitely want to 
change the design.

The suggestion is simple: abstract form generation from actual output. Generate 
an array of form elements' HTML and later iterate over it or even use something 
like Visitor pattern as QuickForm [1] does.

> If you had to look that hard to find 
> something like this, I'm pretty happy, since no one has given the 
> project a critical review.

Consider this: maybe I just know where to look and what to look for? ;)

>>> 2) Usage of homegrown i18n solution rather than some established ones
>>>   Isn't it obvious from the first glance what this string does?
>>>   {!i18n:visitors:{@visitors}}
>>
> I looked at some of the more popular i18n implementations, and decided 
> that I could more easily implement an xml or pgsql table based 
> implementation.  It's super flexible, and allows for pretty elaborate 
> conditionals.  Perhaps you didn't look at the way the XML structure is 
> set for the i18n tags?  Here's the one for the one you pointed out:

It is impossible to say what string will be output by just looking at this 
placeholder in the template, while it is obvious when using gettext(). As for 
conditionals, gettext allows them, IIRC.

>>> 3) Abstraction of DB API rather than usage of proper data access layer
>>>   This is BTW the reason for poor PostgreSQL support in most PHP 
>>> applications: their authors use such "abstraction layers", but design 
>>> schema for MySQL and then just do minimal "translation" of queries 
>>> they do. Upper levels should know nothing about SQL and just call 
>>> lower level to fetch some data.
>>
> I designed the abstraction layer with PgSQL in mind, and with the idea 
> that someone might want to come in and add support for something else.  
> The way the abstraction layer works doesn't affect performance at all, 
> and allows for some unique methods of querying data in blocks.  It's 
> very easy to code for, and allows for completely different query 
> structures for different backends, while not having to jumble data 
> around.  There was a lot of thought of that went into how the 
> abstraction layer would work, and while it is quite different than the 
> way PHP normally handles queries, there is a reason for the two step 
> approach, that I feel justified in creating.

You didn't understand my point. I mean the following: your upper layer still 
contains SQL queries, here is a typical example from TCMSViewer class:

       $this->Database->AddQuery("key_lookup", "SELECT a.*, b.*, 
to_char(a.created, '" . $this->parent->Configuration->datetime_format . "') AS 
update FROM cms_entries AS a, cms_keys AS b WHERE b.cmskey = a.cmskey AND 
a.cmskey = '$1' AND a.approved = 't' AND a.active = 't' AND a.language = 
'$language';");

As you see, the call only abstracts DB API, it does not abstract actual query 
which contains PostgreSQL-specific to_char() formating function.

If you did the Right Thing, there would be no query here but just a call to a 
hypothetical loadCmsKey() method of hypothetical DAO.

> I've been making a concious effort to go back through and update 6 month 
> old code to reflect how we're doing things now, and to enforce some 
> standards on contributed code.  If you'd like to help clean things up or 
> make some constructive criticism, I'd love to hear it.

Create your own coding standards or adopt existing ones. Make all existing code 
follow them --- there are automatic code beautifying tools available, you know. 
Do not accept contributions not following the standards.


[1] http://pear.php.net/package/HTML_QuickForm




view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [pgsql-www] PostgreSQL.org Design Proposal
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox