pharo-nosql / mongotalk

A Pharo driver for MongoDB
MIT License
19 stars 13 forks source link

Rethink forwarding pattern from MongoCollection to MongoDatabase to Mongo #34

Open tinchodias opened 6 years ago

tinchodias commented 6 years ago

If you see in current master, there are at least 8 cases of this pattern:

MongoCollection>>
commandInsert: newDictionary writeConcern: aConcern
    "Insert using Mongo command. Answer the output of the command.
    See more on https://docs.mongodb.com/manual/reference/command/insert/"

    ^database commandInsert: newDictionary collection: name writeConcern: aConcern

where the API of MongoCollection correctly exposes a behavior of a collection in mongodb, and always the implementations is just "forwarding" (may be not the correct word) the message to the MongoDatabase adding some parameter.

Another example, extended:

MongoCollection>>
insert: aCollection
    database insert: aCollection collection: name

MongoDatabase>>
insert: aCollection collection: aString 
    root insert: aCollection collection: name , '.' , aString

as you can see, the MongoDatabase also forwards the message to root (an instance of Mongo).

This is not only a pattern that means annoying code... IMHO, the fact that a database contains collections doesn't imply that MongoDatabase should be a kind of facade or mediator between MongoCollection and Mongo. I think MongoCollection could send messages directly yo Mongo.

Also, MongoDatabase and MongoCollection belong to a same abstraction level. Only Mongo is more low-level stuff because it reads and writes to the socket that connects to mongodb.

Finally, a proposal to fix it: let MongoCollection know the root (Mongo instance) and not only the MongoDatabase. This:

MongoCollection>>
insert: aCollection
    root insert: aCollection collection: self database name , '.' , self name
noha commented 5 years ago

What is the current status?

tinchodias commented 5 years ago

I still have the same question. Do you have any opinion? I can create PR this week.

tinchodias commented 4 years ago

Update: Now that I worked on the MongoClient (in #68), I suspect MongoDatabase and MongoCollection fit much better at the level of this SDAM package instead of the more lower-level Mongo model.

noha commented 3 years ago

Can this be closed?