kwanste / cse403-wi11-wikimap

Automatically exported from code.google.com/p/cse403-wi11-wikimap
0 stars 0 forks source link

cacher.php and retriever.php code review request #147

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
There is a need to refactor the code in retriever.php.  The code right now is 
not a good api experience for any developer that chooses to use this 
constructor.  An ideal implementation should be that his knowledge is only of 
the parameters that go into __construct.  The current code requires editing of 
the private variables above the constructor and learning what the conditionals 
in the implementation are in order to create a custom connection..

See attached screenshot.

This also makes it difficult to write the release notes.

Original issue reported on code.google.com by kwanste on 11 Mar 2011 at 4:07

Attachments:

GoogleCodeExporter commented 9 years ago
I'm not sure what you're referring to. The constructor *is* missing access to 
custom username/password, but it doesn't require you to edit any other private 
variables. Those are default parameters in the constructor - if you don't 
specify them, it falls back to what comes after the =. No editing of $server or 
$db.

Original comment by dylan.ho...@gmail.com on 11 Mar 2011 at 4:21

GoogleCodeExporter commented 9 years ago
Cacher has a similar issue.  These should be abstracted into one php script, 
and in retriever.php and cacher.php - defaults should be defined at the top of 
the file.

Original comment by kwanste on 11 Mar 2011 at 4:24

Attachments:

GoogleCodeExporter commented 9 years ago
From the perspective of a developer who is trying to create his own defaults 
for his own custom database, he would have to go and figure out which of these 
parameters he would have to replace with 1) his own default server address, 2) 
his own default username, 3) his own default password, and 4) his own default 
database.  

Original comment by kwanste on 11 Mar 2011 at 4:27

GoogleCodeExporter commented 9 years ago
Confirmed and updating to be more specific.  To enhance our shipping code's 
quality, we should ultimately find a better representation for this, but we 
will not worry about it in the final timeframe.  Will revisit in the future.

Original comment by holycrap...@gmail.com on 11 Mar 2011 at 4:35

GoogleCodeExporter commented 9 years ago
Fair enough, easy fix though - just get rid of default parameters and make them 
make it explicit.

Original comment by dylan.ho...@gmail.com on 11 Mar 2011 at 4:38