-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hey Matt,

> - It desperately needs an option for padding.  Having everything butt up
> against the left or right (if not centered) doesn't look quite right.
>  I'd set the default padding to 1, so that things look like this:
> 
> +---------+--------+
> | Date    | Rating |
> +---------+--------+
> | 2008-06 | 5      |
> | 2008-07 | 6      |
> | 2008-08 | 8      |
> | 2008-09 | 0      |
> | 2008-10 | 2      |
> | 2008-11 | 6      |
> +---------+--------+
> 
> instead of like this:
> 
> +---------+--------+
> |Date     |Rating  |
> +---------+--------+
> |2008-06  |5       |
> |2008-07  |6       |
> |2008-08  |8       |
> |2008-09  |0       |
> |2008-10  |2       |
> |2008-11  |6       |
> +---------+--------+

Padding sounds useful, will add that.

> - I'd replace the second two parameters of the constructor with an
> options hash, which allows you to set values for separation, decorator,
> and padding.  For example,
> 
> $options = array(
>     'separation' => 'header',
>     'decorator'  => 'Ascii',
>     'padding'    => 1
> );
> $table = new Zend_Text_Table(array(9, 8), $options);
> 
> as opposed to:
> 
> $table = new Zend_Text_Table(array(9, 8),
> Zend_Text_Table::AUTO_SEPARATE_HEADER, 
>                              new Zend_Text_Table_Decorator_Ascii());

Good idea, will do that.

> - There should also be mutator methods for each of these options (i.e.,
> setSeparation(), setDecorator(), and setPadding()).

Will be added as well.

> - In keeping with other areas of the framework, I should be able to pass
> in the strings 'Ascii', 'Unicode', or 'My_Text_Table_Decorator_Whatever'
> as the value for the decorator, in addition to an actual object.  The
> common use case is switching to 'Ascii', and having to type "new
> Zend_Text_Table_Decorator_Ascii()" is a bit sadistic.  :-)  I recommend
> using Zend_Loader_PluginLoader for this.

Makes sense, I will look at some other component and see how to
implement that (but yeah, I know that I am sadistic ;)).

> - It's a simple component, but it needs more documentation than it
> currently has (with more code examples).  You have nothing about
> separation or decorators, for example.

At this point I would be glad if you could help me out, I'm not that
kind of a long documentation writer :)

> I think the options hash for the constructor is the most important of
> these in the short term, because changing it in the future will be a BC
> break, whereas the others are improvements.  Second on my wishlist would
> be padding.  If you would like some help to get them done before the
> deadline, contact me off list and I'd be happy to lend a hand.

You'll get both, thanks for your suggestions!

> Other than those few things, I think this is a great component.  Good
> job, Ben!
> 
> -Matt

...................................
:  ___   _   ___ ___ ___ _ ___    :
: |   \ /_\ / __| _ \ _ (_)   \   :
: | |) / _ \\__ \  _/   / | |) |  :
: |___/_/:\_\___/_| |_|_\_|___/   :
:........:........................:
: Web    : http://www.dasprids.de :
: E-mail : [EMAIL PROTECTED]       :
: Jabber : [EMAIL PROTECTED]     :
: ICQ    : 105677955              :
:........:........................:
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkYk30ACgkQ0HfT5Ws789A0rwCfYbPjeSi9YicpJD9Lznsc3uYx
QDgAn2XeXNKdsrU5MKUWXTZTXLMn8gwg
=wrNt
-----END PGP SIGNATURE-----

Reply via email to