-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 OK, I'm just going to go through all the bad stuff I see in Blue, because that's what I'm good at. Sorry if that seems harsh.
Overall ======= Overall I would say P5EEx::Blue's biggest problem is that it is entirely designed from the perspective of web development, and I think it suffers because of that. Generally it's great that this has got off the ground, but now a lot more work needs to be done - this isn't even ready to be 0.01 yet - if we released this to the world at large the J2EE community would laugh at us. Config ====== o get_branch should return a new Config object, not a hashref. o There is no reload() method. o There are no setters. Context ======= o Context knows *way* too much about widgets. It shouldn't know anything at all about them. Put that stuff in a subclass if you have to. o There seems to be a lot of "convenience" methods in there. I think this is confusing right now. o There's both debugging and logging. Why separate the two? Why are they even in this class? LogChannel ========== o There is only a single log level. This is fatal. See Log::Dispatch (and Log::Dispatch::Config) for a better design. Messaging ========= o I don't know enough about messaging APIs to really comment on this. I get the feeling that the API is naive, but it's merely a gut feeling. Procedure ========= o Do we need this class? I'm not convinced we do. It seems again like a naive design - I'd like to see what j2ee does in this space. Repository ========== o This seems to be yet another DBI abstraction layer. It seems quite tied to the concept of relational databases (or at least - relational data). What are the rules for the $params option? If that's subclass dependant it seems kinda pointless. Security ======== Seems unstarted, which is a shame as this is one of the nuts-and-bolts parts of an enterprise environment. Serializer (ugh, american spelling ;-) ========== o We need a facility to serialize directly to a filehandle (even if it's implemented as a serialize to string and then writing to a file), and directly to callbacks. Session ======= o html() method *must* die. o This seems like a place where p5ee could be really strong, but it's not. Sessions require facilities like timeouts, getters/setters, and a few more bits. SharedDatastore =============== o "A SharedDatastore service represents a single hash in which scalars or deep references may be stored. (They are automatically serialized using Storable for storage.)" - Why are they not serialized using Serializer? Or is this a doc bug? o How do we define where the shared data is, what the limits are, and what timeouts exist? SharedResourceSet ================= o Looks good. No complaints ;-) TemplateEngine ============== o I'm not sure I like this, or even think it should be in p5ee. Templating often works extremely differently depending on the environment you're in, and the templating system in use. One of the biggest plusses for XSLT (for example) is that the result of an XSLT transformation is a new XML DOM tree, not a string, as you have it in your API. I'd say leave this out of p5ee. Widgets ======= Ah, the widgets... I have a feeling that this is where you spent most of the development effort. I've spent a lot of time studying and trying different techniques of doing cross-environment widgets, and it just plain don't work. Right now all you have is Widget::HTML::*. When you get to trying to do Widget::Gtk, or supporting some of the Gtk features in HTML, and trying to be a transparent layer between Gtk and Tk, you'll see it just doesn't and can't work. These different layers are so fundamentally different that no layer over the top of them can just make them work. While I think the Widgets stuff is an excellent separate project, I think it has no place in p5ee, unless everyone thinks that p5ee should be focussing on web development? Docs ==== The docs are nice (albeit lacking detail). Good work on those. Conclusions =========== I hope I haven't been too harsh. All in all I'm +1 to make P5EEx::Blue the start of P5EE (bet you weren't expecting that!). The reason being that we need to start somewhere, and Stephen obviously has a lot of energy for this project where the rest of us don't. But I think it's going to need major surgery, and a serious effort from more than just Stephen (and no, that's not me volunteering ;-) - -- <:->get a SMart net</:-> -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.7 (GNU/Linux) iD8DBQE84hg0VBc71ct6OywRAqg2AKDQjzoJD/oL5IaJZL4zeICS+2yypQCgiS6P zckeIhTFnCylAzYR4eXaOcs= =M0mf -----END PGP SIGNATURE-----
