0

I am currently working on a new project for university and was curious as to the best way to handle a class I've created to perform utility operations such as hashing passwords.

Should the utility class contain static methods so that I call them like 

Utilities.hashPassword(password,salt);

Or should I create a new instance for each call

new Utilities().hashPassword(password, salt);

Right now I have a new instance for each call to a function inside that class, but im concerned about the performance implications of this and am wondering if its even nessecary to do.

My original reason for instantiating them was because I wasn't sure how thread-safety worked and was concerned that multiple users calling the same static function would cause problems. After reading some material on java concurrency I'm now pretty sure that even if the method is static it would be thread-safe.

Should I change them all to static methods? Would this improve performance? Right now my test server buckles under load.

Thanks

FMC
  • 650
  • 12
  • 31
  • What sort of load balancing are you doing on your server? – Tim Biegeleisen Mar 24 '15 at 10:03
  • All methods static and make the constructor private to ensure it cannot be instantiated – alainlompo Mar 24 '15 at 10:11
  • 1
    Avoid utility classes if you can: they are a pain. `new Utilities().hashPassword(password, salt);` could easily be: `new PasswordHasher().hashPassword(password, salt);`. Then you have a nice class with clear responsibilities: not a dumping ground. – David Lavender Mar 24 '15 at 10:19
  • 1
    I obviously can't be certain without seeing the exact case and proper measuring but I would be very surprised if the instantiation of utility classes caused so much extra load. (It is possible, I've seen it happen, but usually there's something else.) – biziclop Mar 24 '15 at 10:20

2 Answers2

1

Thread-safety does not care if a method is static or a true member method. Thread-safety cares about concurrent modification to data. So, if your method is updating some generic data structure, you are NOT thread-safe just by making it static.

Arguments against "static": anything that is static is very hard to mock within unit tests. So be really careful about making stuff static just for convenience.

Regarding the performance aspect: object creation is very cheap in java (not completely free, but cheap). In your case - you could keep it a member method - just avoid to throw away your utility object all the time.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
0

Should I change them all to static methods?

Yes. Utility method should be static. Because, instead of new Utilities().hashPassword(password, salt); use only static import of hashPassword(password, salt). Shorter and easier to read.

Would this improve performance?

Yes. Declaring static will save memory. It will also improve readability.

See also: Java: when to use static methods

Community
  • 1
  • 1
Masudul
  • 21,823
  • 5
  • 43
  • 58
  • 1
    ...and worsen testability – NeplatnyUdaj Mar 24 '15 at 10:05
  • But keep in mind, that mock testing your code with static methods can be quite annoying... – Ria Mar 24 '15 at 10:05
  • @Ria but keep in mind, that not every method needs to be mocked. In fact it's a lot easier to write tests that don't mock things, and the test is no less useful for it. (The exception is things you can't practically control in the test, such as external web services). – user253751 Mar 24 '15 at 10:08
  • If the created Utilities object is not discarded but reused, there is no saving of memory compared to static methods. – GhostCat Mar 24 '15 at 10:08
  • 1
    Downvoter, care to comment please, without comment it will not help anyone. – Masudul Mar 24 '15 at 10:09
  • @immibis, that's true. I'd say the answer depends very much on your choice of framework. I wouldn't use static service methods if I have a dependeny injection framework like spring. I would prefer creating an instance for it. – Ria Mar 24 '15 at 10:10
  • @immibis The point is that Masud gave a **generic** recommendation: it is not **always** a good idea to make utility methods static. If such a static method does a lot of things ... and this static method is used in code you have to test ... then you have a lot of "fun". – GhostCat Mar 24 '15 at 10:10
  • @EddyG and if the static method does one thing (like hashing a password), and has been tested (if you're doing them; anything related to testing best practices is out of scope of this question) then you won't. – user253751 Mar 24 '15 at 10:13
  • @immibis Again: I am not saying that static methods are always bad. I am only objecting the implicit statement that they are **always** good. – GhostCat Mar 24 '15 at 10:14
  • @Masud Not sure why you are complaining - there were already two comments pointing out potential weaknesses in your answer. But just to be sure you understand why I downvoted: you make a **general** statement "Utility methods should be static". If you turn that into "in this case, it seems reasonable to use a static method" ... i don't mind upvoting. – GhostCat Mar 24 '15 at 10:19
  • @EddyG, Although there may be some weakness of static method but there is also some strong point to use static method especially for Utility class that I have pointing on my answer and also show the link. – Masudul Mar 24 '15 at 10:33
  • 1
    I think you are still postulating opinions as facts; for example there are many people who absolutely not use static imports. This is a matter of style and personal taste. You found one posting that says "static methods are OK"; well I got one that says "static is bad" (http://stackoverflow.com/questions/17380348/creating-static-utility-methods-should-not-be-overused-however-how-to-avoid-it) ...now what? – GhostCat Mar 24 '15 at 10:40
  • I am a self-confessed static-impot-non-user. I think they're a bane on readability. – biziclop Mar 24 '15 at 11:04