Re: [Tsheetx-developers] 2.0 demo feedback

[Scott M. <sco...@gm...>]

Thanks for the link for various reasons for OO encapsulation.  The points
that were particularly swaying to me were:

   1. You've insulated your public interface from changes under the sheets.
   2. You can hide the internal representation. getAddress() could actually
   be getting several fields for you.
   3. Inheriting this class, you can override default functionality.

having gone through several iterations of changing needs/ideologies in my
career, the ability to change things under the sheets, yet allowing the
original code to continue to work...  that's good engineering.

-Scott


On Mon, Feb 14, 2011 at 6:03 PM, Mark Wrightson <ma...@rw...>wrote:

>  Hi Scott,
>
> Don't worry, i won't take things personally, it is for the good of the
> project at the end of the day.  I have been playing around with wordpress
> recently and here is their config table:
>
> CREATE TABLE IF NOT EXISTS `wp_options` (
>   `option_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
>   `blog_id` int(11) NOT NULL DEFAULT '0',
>   `option_name` varchar(64) NOT NULL DEFAULT '',
>   `option_value` longtext NOT NULL,
>   `autoload` varchar(20) NOT NULL DEFAULT 'yes',
>   PRIMARY KEY (`option_id`),
>   UNIQUE KEY `option_name` (`option_name`)
> ) ENGINE=InnoDB  DEFAULT CHARSET=utf8 AUTO_INCREMENT=376 ;
>
> Now the thing that i think is an improvement here over your proposed design
> (the only thing actually) is
> the option_value is of type longtext and there is only one column for value
> whereas you have three different fields for int, float and text.  Ultimately
> an int and a float can be stored as text and it would make no difference to
> the end result.  php decides by itself the type of a variable anyway.  By
> changing your config to one field it would im guessing simplify the php that
> has to load everything in?  or... is there good reason to have three
> different fields in the db?
>
> In the config stuff in the demo, there really only needs to be one
> additional function (a getter), there is no real need for a setter as a
> config item shouldn't be changed by the system.  There were a few setter
> functions which have now been removed when I consolidated all the config
> into config.factory.php, config.class.php and config.php.
>
> With reference to why you should use getters and /or setters, I have found
> a nice article on stackoverflow.  It relates to python but the article is
> still 98% valid.  There is more to it than just sloppy code.
>
> http://stackoverflow.com/questions/1568091/why-use-getters-and-setters
>
> An example in the current tsng code where a getter is very useful is the
> Config::getWebmasterEmail() function.  The code originally just returned the
> email address until I realised that it was much better practice (for spam
> reasons) to encode the email first.  The encoding makes no difference to the
> programmer or end user and it meant only a single line change rather than
> having to change every call to the variable in the project.
>
> I do understand where you are coming from in regards to needing to add more
> code everytime a new config item is introduced, but it does make for a more
> robust system at the end of the day.  I guess there is one alternative
> (albeit not my favourite idea), it is possible when doing an sql query to
> return an object instead of an array.  The object is of type stdClass and
> the member variables are all public.  It is an alternative, but personally I
> would not recommend using it for data that needs to be passed around every
> class file in the project.
>
> The place where I now use the sql return as object command is when grabbing
> data (for example) to display tables of information.  The file
> database.class.php file demonstrates this quite neatly.  MySQLDB::sql()
> line 158  - mysql_fetch_object($result)
>
>
> Regards
>
> Mark Wrightson
>
>
>
>
>
> _____________________________________________
>
> Mob:  07725 695178
> Email: ma...@rw...
>
>
> On 14/02/2011 23:00, Scott Miller wrote:
>
> Absolutely, please don't take anything personally, I just want it to work
> and work well.
>
> The one line change to the CSS worked a treat.  Much better :-)
>
> So, ok, I'm not totally convinced of the OO stuff, so, please bear with
> me.  With my class to read in and store the configuration information, it
> doesn't matter what is added to the configuration table in the database
> everything is read in, and the class doesn't need to be edited every time
> something new is added.  Only when and where that new thing is needed do we
> need to change code.
>
> In the version being championed by Peter and Mark, every new config item
> needs two more functions to be added to the class, on top of the additional
> code surrounding that item.
>
> I guess I'm more of a minimalist, I don't feel the added layer of extra
> code, and extra function call for each config data look up, is a good trade
> off to ensure against sloppy code.
>
> -Scott
>
> On Sun, Feb 13, 2011 at 12:40 PM, David Thompson <
> tom...@us...> wrote:
>
>>  Guys, try to keep your exchanges on the developer-list (that means
>> checking the addressing when you reply).
>> That way there is a (searchable) record for everyone to see what is going
>> on.
>>
>> Critisism should be nothing personal (hard on the product, but soft on the
>> person), in that way it can only make the software better.
>> Cheers
>>
>> ------------------------------
>> Date: Sat, 12 Feb 2011 19:45:43 +1100
>> From: pal...@gm...
>> To: sco...@gm...
>> CC: ma...@rw...; tom...@us...
>> Subject: [SPAM] Re: 2.0 demo feedback
>>
>>
>> Scott,
>> I agree also with this issue. I think it can be solved by continuing in
>> the direction we are going. That is, to have minimal html structures and
>> minimal html tables, and to use css includes to do all the formatting. I am
>> not a css expert. In fact I would rate my css skills at below beginner.
>> However the book I am reading, "CSS The Definitive Guide" by Eric Meyer
>> (O'Reilly) has numerous examples where the styles are set up to allow
>> expansion and contraction of table cells etc. So I have confidence we can
>> fix the problem you raise.
>>
>> The next phase of the presentation is now to devise and test css styles to
>> consistently format the monthly, daily, weekly and simple time entry tables.
>> But that will take some time to organise.
>>
>> I would also like to "modernise" the menu system into a set of tabs, or
>> maybe a hierarchical directory tree on the left.
>>
>> Finally, I'm soon to take off on a trip to colder places late this month
>> and next, returning around 17 March.
>>
>> Ciao
>> Peter
>>
>> On 12/02/11 07:51, Scott Miller wrote:
>>
>> (finally changed the subject)
>>
>> Another complaint:  We need the site to be able to (mostly) fill a browser
>> window.  I have a large screen, and it irritates the begeezers out of me
>> when I have a site that can only fill a small portion of my available
>> screen.  This is especially important for the simple timesheet where you
>> have a potentially lengthy description field that suddenly becomes nearly
>> unusable because the space available is tiny.  Configurations, user mgmt,
>> reports, all these need the ability to expand horizontally when they can.
>> This may not be as important for all areas, but I think it would be bad form
>> to allow variable pages only in a few areas...
>>
>> -Scott
>>
>>
>> On Fri, Feb 11, 2011 at 8:24 PM, Scott Miller <sco...@gm...>wrote:
>>
>> Hey guys,
>>
>> Ok, admittedly, I'm not an OO guru.  But, could you please explain why the
>> table name class has been created in the way that it has?  In particular,
>> I'm totally failing to see how the many tiny routines to set and return the
>> private variables is a good idea.  Why not declare the variables public, and
>> allow the other routines to access the table class variables directly?  And
>> we could have the class itself, read an appropriate .ini file to load the
>> variables in the first place, rather than just including it as a php file as
>> is currently done.
>>
>> Now if there were reasons to protect the members of the class, perhaps
>> some of this is needed for the authentication piece, but I honestly can't
>> come up with a good reason that most of the classes' variables couldn't be
>> public.
>>
>> -Scott
>>
>>
>> On Fri, Feb 11, 2011 at 7:38 PM, Scott Miller <sco...@gm...>wrote:
>>
>> Hey Peter, Mark,
>>
>> I just downloaded (checked out via svn) the 2.0-demo branch.  I haven't
>> played with it a lot, but for the most part it looks good.
>>
>> I do have to complain about the new date navigation.  It looks ok, but
>> it's a pain to use.  I now have between 5 and 10 clicks of the mouse to
>> change the date. That is, in my opinion, way too many; so I think that whole
>> piece needs to be rethought.
>>
>> I'm going to go check out some of the code changes now, and maybe see if I
>> can't add the new configuration stuff into your 2.0-demo...
>>
>> -Scott
>>
>>
>>
>