Is good practise pass cursor to constructor? (Java/Android)

Question

Some my friend tells me that initialization of an object by cursor in a constructor is bad practise. But I don't sure. I initialize cursor and create objects in a one method (I don't pass cursor to another method). In constructor, I also don't move cursor to some position.

Cursor cursor = getVehiclesAsCursor();
while (cursor.moveToNext()) {
  Vehicle vehicle = new Vehicle(cursor);
  //Do something
}
if (cursor!=null) close.closeCursor();

My constructor in Vehicle class

public Vehicle(Cursor cursor) {
  id = cursor.getInt(cursor.getColumnIndex(_ID));
  name = cursor.getString(cursor.getColumnIndex(NAME));
  ...
}

Tell me please, is code good or bad? Thanks!

score 1 · Answer 1 · answered Mar 24 '15 at 09:35

I would consider this bad practice for the following reasons:

A cursor is a short-lived construct, that is only valid within a transaction. If the object lives longer than the transaction, and keeps a reference to the cursor, it has in the best case a stale cursor, and in the worst case messes with the transaction handling
A cursor is a highly implementation specific object. In your example you create a dependency on the data object to the SQL logic. Why does a data class need to know how to read data from a cursor, handle SQL errors etc? All it needs to know is its data.
This is an extension of point 2: You tie your data class to your database implementation. If you change the way you store your data, you'd have to change the data classes. You don't want that.

Thanks for your reply! I'm not save cursor in object, I fill object from cursor and nothing more. But.. What about 2 or 3 ? Do I need in setters methods and fill object ? Is it better? — Mikhail_dev, Mar 24 '15 at 12:44
You should definitely create setters for the data attributes (unless this a immutable object). How to get the data from the cursor depends on the circumstances. A good way would be a factory method, maybe in a factory class, something like: VehicleFactory.createFromCursor(Cursor c) — Thomas Stets, Mar 25 '15 at 06:12

score 0 · Answer 2 · edited May 23 '17 at 12:12

This depends on the purpose of the object and the lifetime of the object. If the lifetime of the object need not exceed that of cursor, it may be ok, for example, in What's the best way to iterate an Android Cursor? there are two solutions that construct an Iterable to use a for(:) loop on cursor, in that case passing a cursor to such object is inevitable.

Another question is whether is is good practice to store in an object a cursor that may get closed or moved during object's lifetime. Since this would be a side effect, the answer is no. Even if your code does not move/close the cursor now, someone (probably you) may later modify the code and face a bug -- or, even worse, let the user face this bug.

Now, what arguments people expect to see in a constructor? Usually, the data that get stored in the object's fields. You are not going to store the cursor in a field. What code people expect in a constructor? The code necessary to create internal data structures. If the data structures are simple, people expect the constructor to be simple.

For this reason in my code I have a static method MyStuff.fromCursor(Cursor cursor). This method is responsible for initialization of the object, but not for its construction.

Is good practise pass cursor to constructor? (Java/Android)

2 Answers2