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

Client isn't really very Go like #1

Open
crzyuk opened this issue Jul 20, 2024 · 3 comments
Open

Client isn't really very Go like #1

crzyuk opened this issue Jul 20, 2024 · 3 comments

Comments

@crzyuk
Copy link

crzyuk commented Jul 20, 2024

Hi,

Having tried the client I have to say this isn't really written that well and it's not that nice to use.

Firstly you rely on env vars for tokens which is generally unsafe.

Secondly, the client init is automatically searching for an env var so you get an error even if you attempt to use the NewClient function with a token fetch from a secure source.

Thirdly the client is some strange global variable which isn't that nice to be honest. If you're creating a client with a token as above you end up with a strange floating variable that you have to do something with to avoid linting errors and trying to test things with a hidden global variably is virtually impossible..

Not sure what the aim was here?

Don't get me wrong, the API is great but this client isn't that great from a Go point of view.

@MarketDataApp
Copy link
Owner

Thanks a lot for the feedback. This was my first attempt to make something in Go, so I appreciate the suggestions.

I'll try to put some more work into this next week to address the issues you brought up.

Feel free to make any other suggestions as well and I will try to take them into account.

@crzyuk
Copy link
Author

crzyuk commented Jul 20, 2024

No worries at all. Thanks for the reply.

If I have time over the next week I'll try and submit a PR to make it a bit more usable - if that sounds ok?

@MarketDataApp
Copy link
Owner

Great, sounds great. I'll wait on your PR before making any further fixes/updates then, so we're not overlapping one another.

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

No branches or pull requests

2 participants