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

More Intuitive #10

Closed
littletrainee opened this issue Apr 18, 2023 · 6 comments
Closed

More Intuitive #10

littletrainee opened this issue Apr 18, 2023 · 6 comments
Assignees

Comments

@littletrainee
Copy link

Should we change the variable names to 'Item'? This way, it would be more intuitive to know what is inside the 'Pair' or 'Triple' structs. Should we also change the package name to 'PairAndTriple' to make it more straightforward?

@DmitriyMV
Copy link
Member

Greetings! Thanks for the interest in gen module :)

I think PairAndTriple package name looks kinda un-goish. Also this would forbid us from adding things like Quartet in future if we want to.

Item1 and Item2 does sound like a good idea from clarity standpoint, but in reality I wanted something very similar to _1 and _2. But, since Go require public fields to start from capital latter, I had to settle for F1. I agreed that naming is not really consistent right now.

Thinking more about that, I'm not sure that pair package makes a lot of sense in the current design. The actually used package is ordered, but then again only because it provides comparators out the box. The pair can be replaced with anonymous structs most of the time, even if it will look a bit ugly.

If I was to redo package currently, I would hide fields behind Unpack() (T1, T2) method and provide a helper function to extract the first or second result. Like Pair_1[T1 any, T2 any](v T1, v T2) {...}. So in the end the usage would look like Pair_1(somePair.Unpack())

@littletrainee
Copy link
Author

Okay, I was originally trying to express the functionality of this module in a simpler way, but maybe that idea is not the most appropriate. If there is an explanation for this function, it might make it easier for people to understand the functionality of this module. The reason for suggesting this is because I saw the Pair section in c# and thought it might be useful to integrate into this module.

@DmitriyMV
Copy link
Member

DmitriyMV commented Apr 20, 2023

You can search github.com/siderolabs/talos for ordered. That should good overview of how it used.

@littletrainee
Copy link
Author

I think you misunderstood my original intention. Your suggestion of implementing the pair and triple functions is good, but it serves a different purpose than what I was originally asking. When searching for Go packages, I came across your implementation of the Pair and Triple functions, which is great for people familiar with object-oriented concepts since they see these as data structures that may contain more than just basic data types, possibly even other structs. However, I wanted to suggest simplifying the module's functionality. While my suggestion may not be the best, looking at the common practices in other languages or other people's implementations may lead to stronger code.

@DmitriyMV
Copy link
Member

To elaborate on my earlier point, the pair package was developed as an afterthought. Its purpose was primarily to replace instances of struct{ something Something; AnotherThing ThingType} in our code. However, there wasn't much thought put into its design and, frankly, I'm still not convinced of its usefulness.

I'm not a fan of the public mutable fields in the pair package - it kinda goes against the whole idea of using tuples. Go already has tuples in the form of multi-value return, which is both safe and what most Go developers would expect. Unlike C# or Java, Getters and Setters are not commonly used and are often discouraged in the Go community. But I guess we're stuck with current pair for now since it's used in a lot of places of our internal code and and that would require a lot of changes just to change internal naming.

Don't get me wrong - I'm not trying to discourage you. In fact, we welcome PRs and we even have the tuple package name available. If you have any ideas on how to implement this, feel free to give it a shot. We can work together to come up with a design that we all agree on and then we can deprecate the pair package.

@littletrainee
Copy link
Author

Perhaps my personal habit is to modify the content of previously designed packages in a timely manner to fit future flexible modifications. Of course, the name and design of each package is a somewhat difficult issue and cannot be modified casually.

However, thank you for your response. Currently, I am focusing on using Go to develop games. I have searched for similar things on package platforms with like-minded individuals, so that I can avoid reinventing the wheel as much as possible. However, what I need more is a simple data aggregation structure. Perhaps I will study more deeply when I have a need or interest in the future. Before that, I will summarize the concepts based on my experience and philosophy of using most programming languages and look forward to finding modules that better fit the same concepts in the future.

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