On Wed, 15 May 2002, Matt Sergeant wrote: > 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.
I agree. It is very webby. > Config > ====== > > o get_branch should return a new Config object, not a hashref. agreed > o There is no reload() method. > o There are no setters. There are also needs to be a "write" method. > 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? It seems like if Context is going to have a user() method, it should have a group() method. OTOH, I kind of think that a roles-based security system might be better as a security model for P5EE, in which case you wouldn't really worry about 'groups', per se. Anyway, I suspect that user & group stuff doesn't belong in context at all, but in the security classes. I agree that the convenience methods seem a bit confusing. What does "user_agent" offer? Maybe that should be available from a _subclass_ (the ones that deal with a web environment), but it hardly seems necessary for the top-level API. > LogChannel > ========== > > o There is only a single log level. This is fatal. See Log::Dispatch (and > Log::Dispatch::Config) for a better design. Yep, see Log::Dispatch. Ok, I'm biased. But it needs multiple log levels at a minimum, and probably multiple channels. BTW, I've been considering adding channels to Log::Dispatch, if you're interested. Given the way Log::Dispatch is constructed, there's no need for something like P5EEx::Blue::LogChannel::Tivoli. You could just do Log::Dispatch::Tivoli. Not that choices are bad, but simplicity isn't bad either ;) > 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. Messaging probably needs some sort of "registry" for senders & receivers. Stem does this, for example. And while you talk about asynchronous messaging, there's really no support in the API for such a thing. I've been doing a fair amount of work with Stem lately, so I've gotten quite familiar with this idea. It's important to realize that you _cannot_ make a synchronous API asynchronous, though you can do the opposite. In other words, the messaging API _has_ to be asynchronous. > 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. I agree. If a widget/service is remote then it should simply magically work. Forcing the programmer to use this class seems to break the encapsulation of RPC. > 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. Please just drop this bit. Basically, some things should not be abstracted. There are important reasons why one might choose to use a RDBMS over LDAP over BerkeleyDB over XML over whatever. The differences in capabilities between these different things is huge, and there is no point in trying to mask this. The current API is _very_ RDBMS-specific, and you will never be able to achieve a meaningful implementation of many of the methods in a non-RDBMS store without basically writing your own RDBMS layer! What may be useful is something less ambitious, like a RDBMS-abstraction layer that works with multiple RDBMS servers, such as Alzabo, Class::DBI, or DBIx::AnyDBD. Although each of those three offers a different degree of abstraction. Similarly, abstracting "key/value" storage mechanisms like the various DB files is probably useful. But trying to write an API that turns a DB file into a queryable data source is an exercise in futility. If you _need_ an RDBMS, you _need_ an RDBMS, and a DB file simply will not do, nor will an LDAP directory, nor will an XML file. > Security > ======== > > Seems unstarted, which is a shame as this is one of the nuts-and-bolts parts > of an enterprise environment. I agree. This could be pretty cool. Like I said earlier, I think that a roles-based system, while more complex, may be a better choice in the long run. And certainly a user/group/perms system can always masquerade as a roles-based system, but not the other way around. > 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. Agreed. > 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. Agreed on both points. Also conditional expiration based on callbacks would be nice. > 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? I'm not sure I understand the point of this class as something distinct from the Session object. Is it something that is used by the Session? > SharedResourceSet > ================= > > o Looks good. No complaints ;-) Agreed > 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. I also agree. I think trying to encapsulate all the possible "templating" engines is hopeless. For example, care Template Toolkit to Mason. Template Toolkit uses a pipeline model of "get data, get template, feed data to template, receive resulting text, output text". OTOH, Mason does something like "start template engine, get data, feed data through template(s) each of which may generate some output". These two mechanisms are completely different, and basically irreconcilable. While Mason _can_ be used as a pipeline, this is generally not how people _want_ to use it, since much its power derives from it particular execution flow. I think a far more useful goal would be to simply provide hooks into P5EE for various template environments. So you'd have Template::Toolkit::Plugin::P5EE, and maybe HTML::Mason::Request::P5EE, and so on. This would let you access P5EE from the templates, as opposed to the other way around. P5EE does not need to Borg-ify everything. Simply making it play nice with a number of template engines would be great. This is much like my argument on data storage. Some things are different for a reason, and any API which masks that difference completely destroys the reasons for choosing one over the other. > 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? I agree with Matt here. Basically, this is again the same point I made regarding data storage and templating. Make P5EE play nice with Gtk, Tk, WxWindows, web servers, etc. But don't try to give them all the same API. This is pointless Borg-ifying. > Docs > ==== > > The docs are nice (albeit lacking detail). Good work on those. Agreed. > 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 ;-) I also agree here. This is a good start, and more importantly the _only_ thing out there. What I'd really like to see is for P5EE to focus on providing services that cooperate with various other environments. The most interesting parts for me are the session, security, shared resource, config, and logging bits. I'm skeptical about being able to make messaging a core service, since an asynchronous API is needed, and this can be quite difficult to integrate with everything else. But I'd be happy to be proved wrong. The data storage, templating, and widget stuff needs to go away. You _will not_ be able to come up with reasonable APIs for these things, and more importantly you _should not_. There are good reasons to choose Gtk over a web page, or vice versa, and there is no way to construct a single app that functions in the same way with both, without coming up with a grossly simplistic Gtk app or a grossly comlicated (and JavaScript-heavy) web app. Similarly, you'll never force Template Toolkit and Mason into the same mold, and that's a good thing. But if I could use P5EE objects _easily_ from my Mason components, that would be pretty handy. Anyway, I say go ahead and make this the P5EE base, minus the templating, data storage, and widget bits. And thanks for all your work on this project. -dave /*================== www.urth.org we await the New Sun ==================*/
