BridgesUNCC / bridges-java

JAVA Client library for Bridges
http://bridgesuncc.github.io
GNU General Public License v2.0
4 stars 12 forks source link

Consider adding a builder class for elements/links #129

Open AlecGoncharow opened 5 years ago

AlecGoncharow commented 5 years ago

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 commented 5 years ago

while it makes sense to follow a pythonic style. we don't want the apis of different language to be too different. let' think though. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

AlecGoncharow commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.