Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove BalanceFlag and use two named arguments in dfpitr_reader() #16

Closed
glourencoffee opened this issue Sep 23, 2022 · 0 comments
Closed
Labels
Priority: Low Issue can be integrated in the next release or later Status: Completed This issue has been solved and can be integrated to development branch Type: Refactor Improvements to the codebase
Milestone

Comments

@glourencoffee
Copy link
Owner

Description

The class BalanceFlag is used as an argument to the function dfpitr_reader() to define which kind of statements should be read:

cvm.dfpitr_reader('file.zip', cvm.BalanceFlag.CONSOLIDATED) # 1
cvm.dfpitr_reader('file.zip', cvm.BalanceFlag.INDIVIDUAL) # 2
cvm.dfpitr_reader('file.zip', cvm.BalanceFlag.CONSOLIDATED|cvm.BalanceFlag.INDIVIDUAL) # 3, default

BalanceFlag is a redundant class, though, since there already exists a class BalanceType. However, using BalanceType in its place would require dfpitr_reader() to take an iterable of BalanceType, which is kinda ugly:

cvm.dfpitr_reader('file.zip', cvm.BalanceType.CONSOLIDATED) # 1
cvm.dfpitr_reader('file.zip', cvm.BalanceType.INDIVIDUAL) # 2
cvm.dfpitr_reader('file.zip', [cvm.BalanceType.CONSOLIDATED, cvm.BalanceFlag.INDIVIDUAL]) # 3, default

It's ugly because there are only two types of balance, individual and consolidated, so taking an iterable makes it seem as though a lot of items are to be passed in to the function, when it will be at most two.

A better approach is to change dfpitr_reader() such that it takes two boolean arguments, individual and consolidated, replacing BalanceFlag:

cvm.dfpitr_reader('file.zip', consolidated=True,  individual=False) # 1
cvm.dfpitr_reader('file.zip', consolidated=False, individual=True)  # 2
cvm.dfpitr_reader('file.zip', consolidated=True,  individual=True)  # 3, default
cvm.dfpitr_reader('file.zip', consolidated=False, individual=False) # 4

Along with the existing functionality of dfpitr_reader(), this refactor will thus allow four possibilities:

  1. Reading only consolidated statements (if present)
  2. Reading only individual statements (if present)
  3. Reading both consolidated and individual statements (whichever are present)
  4. Reading neither consolidated nor individual statements, in which case only the general information about the document is read
@glourencoffee glourencoffee added this to the 0.4.0 milestone Sep 25, 2022
@glourencoffee glourencoffee transferred this issue from another repository Oct 11, 2022
@glourencoffee glourencoffee transferred this issue from another repository Oct 11, 2022
@glourencoffee glourencoffee transferred this issue from another repository Oct 11, 2022
@glourencoffee glourencoffee transferred this issue from another repository Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Issue can be integrated in the next release or later Status: Completed This issue has been solved and can be integrated to development branch Type: Refactor Improvements to the codebase
Projects
None yet
Development

No branches or pull requests

1 participant