Is it safe to return a new object from static method?

Question

Is it OK to have a class something like this

public class Weapon {
 private string name;
 private int might;
 // etc...
 private Weapon(String name, int might) {
 this.name = name;
 this.might = might;
 }
 public static final Weapon Alondite = new Weapon("Alondite", 16);
 // 100++ weapon
}

Then calling the weapon anywhere in the project, like this Weapon.Alondite, will this create new object every time the static (削除) method (削除ここまで) member called?

Or should I do like this, to ensure the object only created once

public class Weapon {
 private string name;
 private int might;
 // etc...
 private Weapon(String name, int might) {
 this.name = name;
 this.might = might;
 }
 private static Weapon mAlondite;
 public static Weapon Alondite() {
 //if (mAlondite == null) {
 // mAlondite = new Weapon("Alondite", 16);
 // return mAlondite;
 //} else {
 // return mAlondite;
 //}
 // EDIT: as suggested by everyone
 if (mAlondite == null) {
 mAlondite = new Weapon("Alondite", 16);
 }
 return mAlondite;
 }
}

• Deduplicator: There is no static method in the first example. And the second has interesting threading behavior.
• bdsl: The first return statement, and the else condition are unnecessary in the last code example. You can just close the if block after the new Weapon line, and then go on to the return.
• Alexander: Side note: return mAlondite is common to both the if and the else. DRY (don't repeat yourself), just write return mAldonite after the if/else, so it's unconditional.
• gnasher729: And "interesting thread behaviour" is not a good thing :-)
• Laiv: If you want it to be singleton and safe, make sure the invariants of the single instance can't be modified. Otherwise, like has been stated, Alondite is just a mere bag of global variables.

Answer 1

No, your first code snippet will not create a new weapon every time you reference it.

The second example you posted is an example of the singleton pattern.

Based on your comment at the bottom of your first example, it seems to indicate that you will have over 100 instances of different weapons. This is a lot. You're right that you don't necessarily want to have more than one instance of the same weapon (one might for different reasons, but your examples seem to indicate singleton-ness).

You might want to consider what might happen if you needed to make another weapon type, or maybe a hundred, or a thousand. You might not want to have to recompile every single time. Otherwise, the purpose of the class might be misconstrued as an weapon type list, not the weapon itself.

Based on your example, it would seem that the 100+ weapon types only differ in name and might. If that's the case, being only different in the data, not behavior, I might consider reading them from a file or something else, and providing a means to look them up (similar to the repository pattern).

public class WeaponLookup {
 private Map<String, Weapon> weapons = new HashMap<>();
 public WeaponLookup(Map<String, Weapon> weapons) { // loaded from file or something
 this.weapons = weapons;
 }
 public Weapon Lookup(String name) {
 return weapons.get(name);
 } 
}

You mentioned in a comment that your weapons will have many attributes, not limited to two. I think this moves more into the favor of using an external, non-compiled data file, as opposed to being compiled into the class.

You can compare something like this

{
 "weapons" : [
 {
 "name" : "Almonite",
 "might" : 16,
 "def-bonus" : 7,
 "spd-bonus" : 9
 },
 {
 "name" : "Alphite",
 "might" : 22,
 "def-bonus" : 2,
 "spd-bonus" : 3
 },
 {
 "name" : "Betite",
 "might" : 16,
 "def-bonus" : 11,
 "spd-bonus" : 4
 },
 {
 "name" : "Gammite",
 "might" : 12,
 "def-bonus" : 7,
 "spd-bonus" : 7
 },
 {
 "name" : "Deltite",
 "might" : 19,
 "def-bonus" : 6,
 "spd-bonus" : 5
 },
 {
 "name" : "Thetite",
 "might" : 11,
 "def-bonus" : 2,
 "spd-bonus" : 11
 }
 ]
}

To something like

public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);

Looking at this example, can you tell which of those numbers belong to which field; is the 11 of Betite spd-bonus or def-bonus? I certainly can't tell. I could go to the constructor and see what it is (it's def-bonus), but I don't really want to do that. If there's going to be 100+, it would be about this long

public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);
public static final Weapon Deltite = new Weapon("Deltite", 19, 6, 5);
public static final Weapon Thetite = new Weapon("Thetite", 11, 2, 11);
public static final Weapon Almonite = new Weapon("Almonite", 16, 7, 9);
public static final Weapon Alphite = new Weapon("Alphite", 22, 2, 3);
public static final Weapon Betite = new Weapon("Betite", 16, 11, 4);
public static final Weapon Gammite = new Weapon("Gammite", 12, 7, 7);

Do you want to release a new version of your program every time you make a change that doesn't really affect functionality? I don't. I'd much rather just have some sort of update checker that downloads the latest version of the data file. Functionality never changes, but suddenly you can add thousands more weapons. Or even removing weapons; if you add a weapon that ends up being more over-powered than anticipated, you can easily just remove the entry from the file; as opposed to building your code again, testing it, and making it available for your clients to download.

• SIRS: Actualy the weapon will have more than 10 attribute, (might, def bonus, spd bonus, etc...)
• SIRS: I decide to accept your suggestion, I will save all weapon in the database, and just create basic class model for the weapon.
• Alexander: To be fair, keyword arguments in other languages, and in Java via IntelliJ, address the latter problem. Being unable to change them without making a new release can still be an issue, depending on whether or not changes are expected to occur
• gnasher729: It’s not an example of the Singleton pattern. It’s an example of the "broken Singleton" anti-pattern.

Answer 2

public static final Weapon Alondite = new Weapon("Alondite", 16);

Then calling the weapon anywhere in the project, like this Weapon.Alondite, will this create new object every time the static method called?

Well no it won't. That's not a method. That's a public member attribute that is stored in static memory. Weapon will only ever be constructed once by this thing. The only "unsafe" thing about this is that it's effectively global.

Your second code listing is just an attempt at the singleton pattern. Which also sets up a global.

Seriously, why don't you just create Weapon once in main() and pass copies of it's reference to anything that needs them?

• Andres F.: The second is a poorly implemented example of the singleton pattern :)
• jwenting: @AndresF. not really. It doesn't try to prohibit instantiation, it only provides a constant sample value for whatever reason.
• Andres F.: @jwenting Hence poorly implemented. It also has concurrency problems.
• gnasher729: And the first is a correctly implemented example of the Singleton pattern.

Answer 3

I'm going to try to answer this from a different angle. It looks like you're trying to create certain instances of weapons with different attributes (and potentially behavior). May I suggest you instead go with polymorphism and instead have something like

public abstract class Weapon {
 private string name;
 private int might;
 // etc...
 public abstract void attack();
}
public class Alondite extends Weapon {
 public Alondite(){
 super("Alondite", 16);
 }
 @Override
 public void attack(){
 //Swoosh
 }
}
public class Kryptonite extends Weapon {
 public Kryptonite(){
 super("Kryptonite", 100);
 }
 @Override
 public void attack(){
 //Boom
 }
}

Anyway, to answer your question, it's perfectly safe to return an instance from static method depending on your intent. Based on your question and everyone else's observation, it looks like you just want one instance of each weapon and you're looking at some sort of a singleton pattern. In that case, and extending from my answer above, change the constructors to private and create a static getInstance method that looks similar yo your second sample. You would want something like this:

public class Alondite extends Weapon {
 private static Alondite instance = new Alondite();
 public static Alondite getInstance(){return instance;}
 private Alondite(){
 super("Alondite", 16);
 }
 // etc...
}

This would ensure that one instance of each weapon is created anywhere because they can only get an instance through getInstance which, in turn gets from a static member.

BTW, there's a threading danger in the way you've written your second sample. If two threads both got into the (alondite == null) block, both would create separate instances of the the weapons. This would mean that the two threads would be pointing to two different instances which is clearly not not what we intend to do.

• Christophe: While in theory you're completely right and this would certainly be the way to go if you'd had a fleet of IoT weapons, this is highly impractical in game development where some creative artists may invent new weapons every day that differ only in some parameters and resource files for the renderer.