[hunchentoot-devel] Patch Submission: Custom Reason Phrases

[Hans Hübner]

Hello Kari,

thank you for your patch.  I have reviewed it, but it is missing
documentation.  In general, it looks a bit over-engineered at the
first sight.  Why exactly is a new custom-status-t class needed?  What
is the use case that it supports?  How does a

(defmethod hunchentoot:acceptor-status-message ((acceptor my-acceptor)
status &key &allow-other-keys)
...)

or even several

(defmethod hunchentoot:acceptor-status-message ((acceptor my-acceptor)
(status (eql +http-bad-request+)) &key &allow-other-keys)
...)

not suffice for your application?  You can easily cover the ugliness
of the specialization syntax with a trivial macro in your application.
 I'm not sure if the use case needs to be supported by fancier
mechanisms in Hunchentoot itself, but maybe some other user here wants
to speak up and support the case?

Please explain your design motivation and rationale a bit more so that
we can understand why the added complexity is worthwhile.

Some source-level notes:

- Avoid abbreviations:  define-make-... instead of def-make-...
- An opening parenthesis is always preceded by whitespace
- An opening parenthesis is never followed by whitespace
- Docstrings are wrapped at 80 characters or less
- Don't refer to other frameworks when explaining our features in documentation
- Don't use ad-hoc reverse hungarian notation (custom-status-T)
- Use generalized booleans unless there is a reason not to (when (>=
status 400) t))
- Don't use PROGN without a reason
- Long lines are not forbidden, but neither are shorter ones if it
makes code easier to read

Sorry to have to reject your patch as it stands,
Hans

On Wed, Feb 1, 2012 at 4:17 AM, Kari Lentz <karilentz at att.net> wrote:
> Included is a patch that incorporates the ability to have a request specific
> reason phrase that allows one to use code such along the lines of
>
>  (setf (return-code*) (make-custom-status 500 "some database query error")))
>
> This would override the templates and the cooked messages but only for that
> request and is intended to be an answer to the "StatusDescription" member of
> the HTTPResponse class in the ASP.NET library.  Essentially, a
> custom-status-t class has been added that links a code with a reason
> phrase.  The "make-custom-status" function above is just a constructor to
> this new class. The "reason-phrase" function is now a generic method that
> dispatches off both the custom-status-t class and the previous numeric
> status codes to be backward compatible.
>
> I also ended up changing the error code dispatch to be CLOS based using eql
> dispatch instead of a case statement for the various http-status-code
> values.  It seems to be a trend to use generic functions instead of a case
> statement in this situation, probably because of a hash table type dispatch
> is used behind the scenes rather than going from start to finish as in a
> case statement.
>
> Cheers,
>
> Kari
>
> _______________________________________________
> tbnl-devel site list
> tbnl-devel at common-lisp.net
> http://common-lisp.net/mailman/listinfo/tbnl-devel