0

I have a library and to create an instance, I use the connectWith() method to send database model:

Wallet wallet = new WalletPoket();
wallet.connectWith(
    DAOFactory.getDAOFactory(DAOFactory.MYSQL)
);

Followed by these methods:

int privateCardId = wallet.addCard(1, "Economy 1");
boolean wasDeleted = wallet.deleteCard(privateCardId);
...

Calling the previous methods will result in a NullPointerException if the connectWith() method is not called prior. Is it possible to force the user to call the connectWith() method or present the user with a warning if they do not? Would it be acceptable to call the method from the constructor?

Wallet wallet = new WalletPoket(
    DAOFactory.getDAOFactory(DAOFactory.MYSQL)
);

What would be the best alternative?

CraigR8806
  • 1,584
  • 13
  • 21
  • 2
    You could throw a more specific `NotConnectedException` and make sure it's outlined that this is how you use your API. I also do think adding it to your constructor would be a good way to go as well. – jacob Aug 31 '16 at 20:01
  • If you're in charge of the library, you can simply throw a custom `IllegalStateException`, **forcing** them to call it. Your library documentation should also explain this requirement clearly. – Passer by Aug 31 '16 at 20:02
  • 1
    Placing it in the constructor is best, preferably as a passed parameter. Otherwise, your constructor is returning a partially initialized object. – bradimus Aug 31 '16 at 20:06

5 Answers5

4

You have a few options.

  1. Force the user to pass the option as an argument to the constructor.
  2. Throw an exception with a message stating that connectWidth must be called if it was not called.
  3. If there is a good default thing to connect with, then connect with that in the constructor.
merlin2011
  • 71,677
  • 44
  • 195
  • 329
1

This is where things get a bit verbose, since it means:

  • You have to check state before you do anything, and
  • You have to guard against developers doing silly things.

One thing you can do is check the state of your connection (ensuring that it's not null), then throwing an IllegalStateException explaining why it blew up:

if(null == daoFactory) {
    throw new IllegalStateException("You are attempting to invoke this without a DAO Factory defined.");
}

...but you'd have to add this check to every method that you had in your program.

A preferred approach in my mind would be to add this to the constructor of the object, since that clearly captures the need to have this dependency before the entity is constructed. So effectively, I agree with your second approach.

The last thing you could do is do some fancy annotation processing to force a specific compiler warning or error should this dependency go missing, but it's likely a lot more straightforward to add it in as a constructor dependency instead.

Community
  • 1
  • 1
Makoto
  • 104,088
  • 27
  • 192
  • 230
0

If the addCard and deleteCard method calls are crucial to the Object's functionality (i.e almost always called), then add it as a constructor.

Otherwise, you can simply throw a detailed IllegalStateException when they're called in the wrong order. You should also document the library methods accordingly explaining what's needed for them to function properly.

Passer by
  • 562
  • 9
  • 14
0

The (library) class Wallet should have provided a constructor that takes the database endpoint. Since it is not available, you could provide a utility wrapper that accounts for it. That way, your utility wrapper can mandate the endpoint and make sure that it is available beforehand.

Another thing you can explore is dependency injection, i.e. whenever a client needs a Wallet, it does @Inject Wallet wallet. This, admittedly, has added complexity, but it renders the code that is more easily testable. (See javax.inject, or dagger).

Kedar Mhaswade
  • 4,535
  • 2
  • 25
  • 34
0

I think, you should implement proxy design pattern to solve the problem. When the client creates a Wallet, they should get the proxy of the Wallet instance.When client invokes the service API say addCard then Real Wallet object instance comes into picture(lazy loading) and does the dao instantiation which is a singleton instance.