search

SciencerIO / sciencer-toolkit

A smarter way to find articles.
MIT License
9 stars 1 forks source link

Consider if `PaperId` should in fact be a class #27

Closed hineios closed 2 years ago

hineios commented 2 years ago

I was looking into this and was questioning if the class PaperIDs makes sense existing as a separate class, or if we should just move it into the paper itself.

_Originally posted by @RicardoEPRodrigues in https://github.com/SciencerIO/sciencer-toolkit/pull/23#discussion_r859599346_

ratuspro commented 2 years ago

The first implementation of Paper had all the available External IDs in the Paper itself. It is also important to note that not all papers have all the external ids available. Decoupling it to PaperIDs helped handling the absence of some external ids and reduced the number of Paper's instance fields.

Right now, I do not see any benefit of merging Paper and PaperIDs.

RicardoEPRodrigues commented 2 years ago

The only current benefit would be a cleaner code, but it might be time and effort better put elsewhere.