Skip to content

FEAT: Add support for additional types #3

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

Merged
merged 6 commits into from
Aug 24, 2023
Merged

FEAT: Add support for additional types #3

merged 6 commits into from
Aug 24, 2023

Conversation

varunu28
Copy link
Collaborator

This code changes comes from the discussion under #1 (comment)

What is changed?

  • Added support for appending Int to StringBuilder
  • Added support for appending Boolean to StringBuilder
  • Added support for appending list of strings to StringBuilder

What didn't work
My initial thought process while approaching this change was to leverage generics. But I made a mistake of thinking that generics support in Golang is equivalent to that of other programming languages. With Go's generics support, something such as below is not possible

func (s *StringBuilder) AppendLine[T any](t T) *StringBuilder

More details: https://www.reddit.com/r/golang/comments/tvfbow/the_fact_that_methods_are_not_allowed_to_have/

Ok. So let's try another approach. Let's just create multiple Append methods with different types. Good old method overloading. Ehhh. Golang does not even supports that in a clean manner. More details: https://stackoverflow.com/a/6987002/3938227

What worked
Adding separate methods for each type does the job. Though it is not a clean abstraction as that of other programming languages such as Java(https://docs.oracle.com/javase/8/docs/api/java/lang/StringBuilder.html), it gets the job done.

NOTE
This is more of a POC. If this looks good, I can extend this PR to support the remaining types as described under the above API spec of Java's StringBuilder implementation

@varunu28
Copy link
Collaborator Author

@linkdotnet I have added a POC. Can you please review this whenever you get a change and let me know what you think about it. Thanks 👍

@varunu28 varunu28 requested a review from linkdotnet August 22, 2023 14:06
@linkdotnet
Copy link
Owner

Can you also add an entry to the CHANGELOG.md? This file is used for creating the release notes. You can see in other entries how it is done.

@varunu28
Copy link
Collaborator Author

Can you also add an entry to the CHANGELOG.md? This file is used for creating the release notes. You can see in other entries how it is done.

Added an entry to the CHANGELOG.md. Let me know if the formatting looks good. Thanks

@varunu28 varunu28 requested a review from linkdotnet August 22, 2023 22:47
@linkdotnet
Copy link
Owner

LGTM! Is there anything open from your side, or can I merge that into the main branch?

@varunu28
Copy link
Collaborator Author

This can be merged and thanks for the detailed review.

@linkdotnet linkdotnet merged commit 0d216b0 into linkdotnet:main Aug 24, 2023
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

Successfully merging this pull request may close these issues.

2 participants