ME1312 / SubServers-2

SubServers – The Minecraft Server Management Platform
Apache License 2.0
91 stars 23 forks source link

A bit of a cleanup for SubSigns #76

Closed Alex1607 closed 2 years ago

Alex1607 commented 2 years ago

Create an SubSigns object instead of using a static class

What's Been Changed

I updated the SubSign Class to no longer be a static construct, but instead use an object-oriented approach. Also I made a bit of a code cleanup.

Why It Was Changed

I thought it's more readable that way and may help newcomers to contribute to the subservers project.

ME1312 commented 2 years ago

Aside from a detour to the nested Text class which has now been relocated, I actually thought the code for Signs was pretty straight-forward in nature as it was... Although, code clarity is kind of a subjective topic to begin with.

2nd opinions?

Alex1607 commented 2 years ago

Aside from a detour to the nested Text class which has now been relocated, I actually thought the code for Signs was pretty straight-forward in nature as it was... Although, code clarity is kind of a subjective topic to begin with.

2nd opinions?

You are correct, code style is a preference. I mean, I could navigate the code quite good while rewriting part of it. For me SubSigns just seemed like a module for SubServers.Bukkit which is why I though a OOP Oriented approach would fit it more. If you don't like the changes, or they don't fit to the rest of the project Style, feel free to close the PR though ^^

ME1312 commented 2 years ago

I'm not really sure why at the last minute I decided I didn't want to store the instance – but you're right – it shouldn't be static, so it isn't anymore. For this particular feature, though, I think I would like to keep it single-class... at least until we get some more module-type features.