opensafely-core / airlock

Other
0 stars 0 forks source link

Refactor DAL side of DAL/BLL interface to use to_dict #376

Closed madwort closed 1 month ago

madwort commented 1 month ago

https://bennettoxford.slack.com/archives/C069YDR4NCA/p1717166090913549

The DAL serialises with e.g. LocalDBDataAccessLayer._filegroup(FileGroupMetadata) and BLL deserialises with FileGroup.from_dict() - is there a specific reason you didn’t go with FileGroupMetadata.to_dict() ? I mean, clearly, the serialiser methods are hidden, but was there a reason to hide them? I don’t have a need to expose them more widely, it’s just that when I see from_dict my first thought is to look for the corresponding to_dict.

I think it would be quite sensible to make them to_dict() methods on the model objects themselves, and no need to be hidden. The only reason they are the way they are was that I was just avoiding making too many structural modifications to the code at one time. - Dave Evans