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

Consider adding a builder class for elements/links #129

Open
AlecGoncharow opened this issue Jan 7, 2019 · 4 comments
Open

Consider adding a builder class for elements/links #129

AlecGoncharow opened this issue Jan 7, 2019 · 4 comments

Comments

@AlecGoncharow
Copy link
Contributor

https://en.wikipedia.org/wiki/Builder_pattern
There is a lot of bridges code that looks like

current = Element()
current.set_value("some_value")
current.set_label("some_label")
current.get_visualizer().set_color("some_color")
my_data_struct.append(current)

or

current = Element("some_label", "some_value")
current.get_visualizer().set_color("some_color")
my_data_struct.append(current)

I think it may be worth looking into a builder pattern for elements at least, and maybe for links/other structures.
It would look something like

my_data_struct = SomeStructure()
my_element = ElementBuilder().value(val).label(label).color("some_color").build()
my_data_struct.append(my_element)

This could be extended easily by adding more functions to the builder. This also could alleviate the requirement for students/users to specify the type of their generic element, by just doing a type inspection on the value, or let them explicitly pass the type to the builder, even as a function if that makes more sense. This ElementBuilder could also be configured to build any sort of Element, possibly using some sort of enum/state based approach so that the builder could handle any linked element appropriately.

@esaule
Copy link
Member

esaule commented Jan 7, 2019 via email

@AlecGoncharow
Copy link
Contributor Author

Well, its a pattern, I have seen it in Java code, including libraries we are currently using in bridges. And its very prevalent in Rust. I would imagine it has been used quite a bit in C++ aswell. It just allows for an easier configuration of complex structures without needing to understand long constructors of objects.

@squeetus
Copy link
Contributor

squeetus commented Jan 7, 2019

I have suggested this style of chained attribute setting for the client APIs before. Historically we have elected not to implement this pattern due to concerns over potential confusion for beginning students (not to mention the added complexity of managing different versions of the same functions or revamping the whole API).

I am personally open to discussing it again for all the client APIs for a future release.

@AlecGoncharow
Copy link
Contributor Author

Sure, which is why the Builder pattern exists, its a separate class entirely, while the existing class can keep its current state, offer a different way of handling object instantiating. I have seen a lot of "Builder" variants in various libraries recently and I just figured it would be worth considering for our uses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants