# Fetching data from your database in a constructor

I have this item (an item is an object linked to a xml view), to build it I need 2 things, get labels from codes, and get text from the strings.xml android file.

please ignore the long to int and int to long convertion, for some reason the database value where randomly set to long and int but they could all be int, I will refactor this later.

refManager is calling a DAOto fetch data from the database.

I have a for loop, building several OperationInstallItem

//constructor

public OperationInstallItem(final Resources resources, final RefManager refManager,
final ModuleCategoryEnum category, final ModeleOperationEntity modeleOperationEntity,
boolean isMandatory) {
//basic init
this.category = category
this.isMandatory = isMandatory;
this.refTypeOperation = modeleOperationEntity.getRefTypeOperation();
this.refTypeInstallation = Long.valueOf(modeleOperationEntity.getRefTypeInstallation());

//label init
this.operationLabel = refManager.getOperationLabel(Ints.checkedCast(refTypeOperation));
this.installationShortLabel = parametrageManager.getInstallationShortLabelt(refTypeInstallation);
this.installationLongLabel = parametrageManager.getInstallationLongLabel(refTypeInstallation);
this.title = resources.getString(R.string.operation_intallation_title)

//icons specific to this class
CodeTypeOperation codeTypeOperation =
CodeTypeOperation.getEnumByCode(Ints.checkedCast(refTypeOperation));
}


Most of my collueages are uneasy with passing a manager as a constructor arguments but noone of them can explain me why. "it's just not done this way"

same thing with Resources resources that allow you to get String values from the strings.xml file doing this resources.getString(R.string.operation_intallation_title), according to them, it is bad to pass it in the constructor.

I don't get it, I need those labels/title, if I do this before the instanciation and pass it as parameters it's the exact same thing. no performance gain, nothing is different.

Or is there something I'm missing?

First of all, it violates the "tell don't ask" principle: if your OptionInstallItem is not inherently coupled to the database and its actions (and as you do not even record the reference to the refmanager, it obviously isn't) it should not go about scavenging in your whole application to get its job done. Always give every class exactly what it needs to operate, not the "master key" for self service.

Then, your way makes it unnecessarily hard to test. If you have a simple object along the lines of

public MyClass(String myLabel) {
this.myLabel = myLabel;
}


Testing is as simple as can be:

MyClass testObject = new MyClass("test");
Assert....


On the other hand, if you have

public MyClass(DatabaseInterfaceThingy database) {
this.myLabel = database.doSomeQueryForMyLabel("me");
}


... say hello to mockito to even instantiate that object.

And send your colleagues over to Codereview to learn a few things and be able to tell you "why" the next time ... ;-)

• allright it's harder to test but is it better to write code in order to make easier unit testing?, I really don't understand how it breaks the tell don't ask principe? where am I extracting data from an object and then making a decision? Aug 3 '18 at 23:33
• @sliders_alpha The object actively asks the rest of the application for data it needs. This makes unnecessary coupling between the object and the other services. And passing in just what you need is not going out of your way for testability (which is in itself a good reason to do it), it is also isolating the object from change. If sometimes you database interface changes, you'll have to rewrite all the objects you created that way. Introducing unnecessary coupling always makes change hard, and code changes over time. Thus, isolation is a good general principle.
– mtj
Aug 4 '18 at 6:09
• for the tell don't ask I still don't understand, I am not asking about internal data (property) of the oarametrageManager and taking a decision, I'm asking for a database value, if I do that outside of the constructor I'm still asking for that data. and for the coupling with the DB, if I do the call before the construction then when the DB interface change I will have to do modification everywhere I'm makine a "new" whereas here I will just have to do it in the constructor, to me it seems like coupling is less important. Aug 4 '18 at 7:28
• @sliders_alpha Sorry, if you think that a widely agreed-upon principle in the software development community is utter bullcrap, I cannot see what I can do to convince you. Maybe this simply comes from experience, so try what you like, get your nose bloodied in a few projects, and then revisit this question.
– mtj
Aug 4 '18 at 7:33
• No I agree with the principle, but not with its interpretation here. the "tell don't ask" principle says that you do not want to use a getter of an internal property of an object to then make a decision. I am not doing this here. Aug 4 '18 at 7:49