On Sat, 4 Jan 2003, James Turner wrote:

> > From: Martin Cooper [mailto:[EMAIL PROTECTED]]
> >
> > Sigh. OK, OK.
> >
> > But three changes I'd like to see in the code you posted earlier:
> >
> > 1) Instead of calling Class.forName(), you should use
> > RequestUtils.applicationClass(), to make sure the context
> > class loader is tried first.
>
> I'll do dat.
>
> >
> > 2) Empty catch clauses are evil. ;-) You should at least log a debug
> > message so that real problems can be debugged more easily.
>
> Well, two of them shouldn't log anything because they are simply there
> to catch the "you haven't got JSTL" case, I though about logging the
> other cases but "they should never happen" (famous last words #43456),
> since it would require there to be a loopTag that couldn't handle being
> sent the messages it defines in the Interface, but I guess I can throw
> in some logging If It Makes You Feel Good :-)

It would make me feel so good - thank you. ;-) One case I'm thinking about
here is when someone finds that it's not working for them, and they
*think* they have JSTL, but it's not being picked up for some reason. A
log message would tell them that Struts tried to do what they wanted, but
something went wrong, hopefully helping them determine that there might be
something wrong with their web app config.

--
Martin Cooper


>
> > 3) Always use braces with if clauses. I know the code has
> > plenty of cases
> > where that isn't done, but it's good practice to do that. I
> > seem to recall
> > Craig admitting that he should have done that in the original
> > code base.
>
> I usually do it to, except when my old c-habits sneak in.
>
> James
>
>
>
> --
> To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
>
>


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to