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

Simplifying modular usage of finance.js, dispensing the main "class" #23

Closed
wants to merge 3 commits into from

Conversation

igorsantos07
Copy link
Contributor

@igorsantos07 igorsantos07 commented Jan 20, 2017

This is related to #22, although it would probably break current usage of var Finance = new Finance();

EDIT: I've added a couple of other functions and changes. If you want I can create separate branches for those so you can merge accordingly - but please, read the commited changes first to see if they fit the project.

@ebradyjobory
Copy link
Owner

Hi @igorsantos07,
Thanks for pointing out the current usage of the class. I definitely agree with what you suggested, but this will take me much more time in order to execute; I will deal with that later. Re: the present value of annuity: this has been taken care of by adding an optional number of periods in the PV function. See previous merge from earlier today. Re: the 100-based rate in PMT, you are right. Please create a new branch with only that change and resubmit a new pull request.

igorsantos07 added a commit to igorsantos07/finance.js that referenced this pull request Apr 7, 2017
…ng Finance both as an instance object and namespace-class
igorsantos07 added a commit to igorsantos07/finance.js that referenced this pull request Apr 7, 2017
…g Finance both as an instance object and namespace-class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants