steinarb commented on code in PR #2807:
URL: https://github.com/apache/shiro/pull/2807#discussion_r3454582497
##########
core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java:
##########
@@ -305,7 +305,7 @@ public Subject login(Subject subject, AuthenticationToken
token) throws Authenti
* @param subject Subject
*/
protected void beforeSuccessfulLogin(Subject subject) {
- Session session = subject.getSession(false);
+ Session session = subject != null ? subject.getSession(false) : null;
Review Comment:
I am partial to using Optional for stuff that require null-checks
I.e. I would have done this like this (yeah, I would have used var for the
local variable as well, but I'm leaving that off for now):
```
Session session = Optional.ofNullable(subject).map(s ->
s.getSession(false)).orElse(null);
```
(I guess it is debatable whether this is more readable than the code from
the PR, but it is the practice I'm following these days. It feels more robust.
But it works better when it is integrated with the API, i.e when the API's
functions returns Optional<something> instead of a nullable something)
https://2022.javazone.no/#/program/7a7de418-d373-415c-8bb9-27cd5cd5900d
(FWIW I've never seen it myself...)
Here is the reasoning behind the talk
https://steinar.bang.priv.no/2021/12/28/how-i-learnt-to-like-optional/
https://steinar.bang.priv.no/2022/01/18/chaining-optionals-using-flatmap-and-map/
--
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]
