Re: [Tsheetx-developers] 2.0 demo feedback

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

Ok, I've been attempting to do two things: 1) figure out where the new
configuration class and code belonged and 2) understand how the loading of
pages through index.php works

If you can shed some light on #2, I haven't gotten very far into exploring
that.

On #1, I'm finally understanding how the existing config class works.
Currently index.php includes the database_credentials file, and then it uses
the class set routines to store the global variables into the class
variables.

What is the vision for what this should look like when we're done?  Should
the class be responsible for reading/including the file?  Should the file
become more of an ini type file?  Or are we happy with just including the
file to create global variables?

I'm thinking if we're going through the work to encapsulate stuff, we would
want the class to be responsible for reading the data, and I would think
making it more of an ini file than one that pollutes the global name space
should be what we strive for, but I also don't want to start working on that
if there's been some decision that says we don't want to do that.

-Scott

On Tue, Feb 15, 2011 at 3:14 AM, Scott Miller <sco...@gm...>wrote:

> 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
>>>
>>>
>>>
>>
>