Investigate - is there any real use case for a DataObjectReplicaCollection class?

[irinaColgiu]

I still think there is no real good use case for this collection. Basically you are re-implementing some of the existing functionality of a simple container in python (list, dict) plus some methods that I am not sure will be used - e.g. will you ever play with a list of replicas and remove from it a Replica object? Very unlikely, mainly because a replica either is there in iRODS or not. And this container reflect what is in iRODS more or less, can't modify what is in iRODS.

Also for get_out_of_date(0) method - if you were to use a plain list, to check whether all objects in a list have 1 attribute set to True/False is just 1 operation that the client can perform easily like:

out_of_date = [replica if not replica.up_to_date for replica in replicas]

I also can't think of a good use case for keeping it as a dict...it's not like you have an array of 1000 replicas in which you need to find quickly something by id. You will always have 1 or 2 replicas for a data object...it looks a bit like an overkill to keep all these methods for a container with at most 2 elements.

[colin-nolan]

I disagree on this one. I think it's helpful to use a specialised container in order to prevent the repeated implementation of domain specific operations on the collection. Although, as you point out, some of the operations would unlikely be performed on a collection of replica on iRODS, there are useful in preparing a model of a collection.

get_out_of_date is just a convenience method to make life just that bit simplier :). There is nothing to stop you still doing the more complex list comprehension that you note, if desired.

I wouldn't really call it "overkill" to store the replica in a dict instead of a list as it's not a much more complicated data structure. Aside from the improved time efficiency, it makes the implementation of methods such as get_by_number and _remove_by_number much simplier.

In summary, I don't think the existance of this collection should really be an issue for the end user. It is a container that is very similar to a Set, with methods to manipulate and access replica with specific properties (e.g. number). In addition, it has domain specific convenience methods and internal validation, not offered by any generic collection.

[irinaColgiu]

OK, let's put it the other way around: do you have a good use case for add, remove, _remove_by_number, _remove_by_object when speaking of file replicas?
There are advantages and disadvantages to have this class there and use it within the DataObject (https://github.com/wtsi-hgi/python-baton-wrapper/blob/master/baton/models.py#L72).

I understand that you may want to use this replicas as a container and I would normally vote for encapsulation, but the logic here I think is already contained in other existing collections.
The disadvantage to having this class used within the DataObject, though, is that you create a circular dependency between collections.py and models.py which I think doesn't "smell" good:

http://programmers.stackexchange.com/a/12030

the reasons listed above don't all apply to Python, as they have been taken from compiled-languages world, but some of them make a good point.
I am not saying you have to absolutely take it out, I am just questioning whether you really have a good use case in mind to make it up for the disadvantage and overhead of understanding the code and dependencies around it.