Re: Please comments on Image_Graph-dev1

Martin Jansen wrote:

On Thu Oct 28, 2004 at 11:4934PM +0200, Stefan Neufeind, PEAR wrote:

at the beginning of this week we've released a first dev-version of the new (!) Image_Graph, based on Graphite. And there were already 94 downloads. Could you please make up your mind about the package and the API, and let us know? In this step we can choose various things between various possible solutions. But I'd prefer the next version to be an alpha - and then we need the API fixed!

I only skimmed through the code quickly, but already have a few questions: Graph.php: . Do you really think it is wise the always log errors into image_graph.log? Some people may want to use another log file and some may also want to use no file-based logging at all.

Whoops, definitely no! Will (have) implemented a setLog($log) method (using a PEAR::Log object or a filename (creating a PEAR::Log object))

. I'd rename hideLogo() to setHideLogo(boolean)

I don't agree - if it'd have to change I don't think at negating setter method is a good choice (i.e what do you have to pass as parameter true to make the logo shown or hidden, well here it's fairly obvious but in general?). Anyway the logo IS shows and I think that it's fairly obvious that hideLogo() hides the already shown logo (no need to do a setHideLogo(false)!), so IMHO the parameter would only leed to confusion. Contemplating on removing the logo entirely.

. It seems to be common practice to use to* methods for output redirection. In this context it may make sense to rename saveAs() to toFile().

Check!

. done() -> get()?

Hmmmm, maybe done() is a bad choice but I do not think get() is better, because I'd expect get() returns the result, but it actually outputs the result (unless you use saveAs()/toFile() or whatever which doesn't return anything either). Maybe display() as in HTML_QuickForm, but then again you could redirect output to a file and then it doesn't diplay, so... Maybe draw()?

I didn't have enough time to look at the gazillion other files, but I'll probably play around with the code later.

Please do!