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

Pointers are vastly over used #11

Open
Bannini opened this issue Mar 24, 2022 · 1 comment
Open

Pointers are vastly over used #11

Bannini opened this issue Mar 24, 2022 · 1 comment

Comments

@Bannini
Copy link

Bannini commented Mar 24, 2022

There is a general overuse of pointers within this library and many make no sense at all (to me).

All return structs fields are pointers to fields, rather than plain fields e.g. string, float or int. This makes this generated code so much less usable as everything has to be nil checked before using. Your own examples using fmt.Printf("%+v", someReturnStruct) produces a nice array of pointer addresses. Look at libraries like AWS SDK, they have made the same mistake and moved away from this thinking. https://aws.amazon.com/blogs/developer/aws-sdk-for-go-version-2-general-availability/

Another example is here: https://github.com/Finnhub-Stock-API/finnhub-go/blob/master/model_earnings_calendar.go#L20

// EarningsCalendar struct for EarningsCalendar
type EarningsCalendar struct {
	// Array of earnings release.
	EarningsCalendar *[]EarningRelease `json:"earningsCalendar,omitempty"`
}

Here we have field that is a pointer to a slice on struct that is a return value. Pointers to slices have their uses (see https://medium.com/swlh/golang-tips-why-pointers-to-slices-are-useful-and-how-ignoring-them-can-lead-to-tricky-bugs-cac90f72e77b ) but as a struct field value on what is essentially api data (i.e. it shouldn't get changed by the application once read from the api), it's pointless.

As an example, perform a query on stock symbols:

symbols, _, _ := finnhubClient.StockSymbols(context.Background()).Exchange("L").Execute()
for _, s := range symbols {
	fmt.Printf("%s : %+v", *s.DisplaySymbol, s)
}

gives a whole array of lines like (one line example)
TIGT.L : {Description:0xc000cb2400 DisplaySymbol:0xc000cb2410 Symbol:0xc000cb2450 Type:0xc000cb2470 Mic:0xc000cb2430 Figi:0xc000cb2420 ShareClassFIGI:0xc000cb2440 Currency:0xc000cb23f0 Symbol2:0xc000cb2460 Isin:<nil>

Please make the library more Go friendly.

@Bannini
Copy link
Author

Bannini commented Mar 25, 2022

Okay - to be fair, I half knew what I was talking about.
The issue comes from the api spec itself, where return structs do not have any "required" properties defined, so everything is seen by the code generator as optional and likely empty.

It would make sense to add required field definitions for many properties of returned object, for example that a StockSymbol call should always have a string symbol associated with it.
I understand that this isn't the correct repo for this kind of feedback so feel free to close with scathing disgust if necessary.

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

1 participant