|
|
|
|
|
by Izkata
667 days ago
|
|
Same here... and funny enough the only thing that really looks wrong to me is they used "this" in only one of the two places they used "sessionProvider". It's not a bug ("this" is implied, so it works just fine), but leaving it out hints to me that it was originally a third argument to "saveUser()", and either the original person writing this didn't have a solid API in mind or "saveUser()" was intended to also be static like "EmailUtil.sendEmail()" and it was switched around later on. Overall hints towards two conflicting styles in the same codebase and no good guidelines. Or maybe there are such guidelines (as hinted in the "solution"), and whoever updated this class only did it partway. This is the point where I'd poke around in the repo history to see why it's like this and whether anything should be changed further in either direction. |
|
Given the Java code in question, while I understand the idea that "well, the session provider may just be hard coded" and such, I would definitely want to hear something about deficient exception handling. It's obviously an important part of the code and while I may not be able to spew out the exact correct exception handling without knowing more about the context and what exceptions may occur, it is obviously very wrong as written.