search

MarketDataApp / sdk-go

Market Data's Official Go SDK
https://www.marketdata.app/docs/sdk/go
MIT License
7 stars 2 forks source link

Client isn't really very Go like #1

Open crzyuk opened 1 month ago

crzyuk commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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