benweidig commented on code in PR #64:
URL: https://github.com/apache/tapestry-5/pull/64#discussion_r3608580359
##########
tapestry-http/src/main/java/org/apache/tapestry5/http/services/Request.java:
##########
@@ -209,6 +209,15 @@ public interface Request
*/
String getRemoteHost();
+ /**
+ * Returns the IP address of the client or last proxy that sent the
request,
+ * always as a numeric address string (never a hostname).
+ *
+ * @return a <code>String</code> containing the IP address of the client
that sent the request
+ * @since 5.10
+ */
+ String getRemoteAddr();
Review Comment:
In general, I totally agree and get where you're coming from, but I would
argue that `Request` is a "consume-only" interface with little reason to
implement yourself.
For example, if `MarkupRendererFilter` would be changed, a lot of code would
break.
But with `Request`, I don't see much risk.
If someone implements it, they are doing some special stuff, most likely in
a library, and then needing to adapt to such a change seems reasonable to me,
or not, as the change is binary-compatible.
However, if you want me to include a `default` method anyway, I would stay
with `String`/`null`, as it's not an optional value according to spec, even
though it's not guaranteed by all environments.
Lately, I try to avoid `Optional`, as it does not provide much utility if it
only represents a singular value that is directly consumed after and not
chained with multiple calls, and simple `null`-check is cheaper.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
