-
Notifications
You must be signed in to change notification settings - Fork 69
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
Stock as module #6
Conversation
I wouldn't use this in production right now. If you like what's going here, lets move all functinality from stocks to Stock and then merge it |
First of all, I think the new const stock = new Stock({
+ "symbol": "MSFT",
+ "interval": "1min",
+ "apiKey": "demo",
+ "datatype": "json",
+ "outputsize": "full",
+ }); No, I meant exactly what you proposed: const stock = new Stock({
+ "apiKey": "demo",
+ }); and then you could call: stock.timeSeries({
+ "symbol": "MSFT",
+ "interval": "1min",
+ "datatype": "json",
+ "outputsize": "full",
}); I don't think you should create a new instance for every request you make. You should only need to create instances for every api key you use. I also don't think it is necessary to support Some of the things I do like are using tl;dr I like the main concept, but readability should be improved first before this gets merged. I'll also create a seperate branch somewhere soon to show you the way I'd do it, and then we can find a way somewhere in between |
No hard feelings. I'll try to explain why I did what I did however it's really up to you. What about readability? TBH I was just releasing my own stockJS written in typescript, esnext and jest. It was on different principles which I like to point here in this PR. |
Im writing from mobile, so ill give a response to your reponse later. But one more thing: i see no integration of the technicalIndicator() function yet or am i wrong? This means it isn't fully functional. Additionally, does the code provided work for the browser as well? So how do you indicate that you want I'd like to use class > prototype as well, as we should stick to ES6 as much as possible. But I want it to be at least as readable as using prototype. I also wouldn't want multiple classes ( |
I dont want to break current workflow so for now I create module as plugin Stock.
import {Stock} from 'stock.js'
const stock = new Stock()
Please note, you can initialize Stock with no options. In that case, defaults settings are passed to constructor.
missing features: