Re: [pgsql-www] PostgreSQL.org Design Proposal - Mailing list pgsql-advocacy

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