Remove Extension Methods
gsulc opened this issue · 4 comments
Extending object is bad form and creates a messy developer experience in any library. Imagine using a graphics library and working in your view-model, or worse: your model! Intellisense would always be populated with graphics-related methods and properties like draw, rotate, x and y coordinates. Additionally, what if a user wanted to separate out their Hubs? Then another developer comes along and uses the object extensions and can't figure out why their message isn't getting through.
One way to achieve what it looks like you're trying to do is to add the following:
private static readonly _hub = new Hub();
public static Hub Default => _hub;like you would for a singleton, but without hiding Hubs constructor. Then, users can use the default Hub, but also create a new one if they need it.
I 100% agree that they are bad practice and would never build something doing it that way today.
However, this is a very old project and removing them outright would be a big breaking change.
We could do a new major version to do that, of course, but docs would have to be updated and such so it's a reasonable chunk of work to get them out of there.
Oh. I saw a commit that was 2 months old, so I thought the project was still somewhat active. Yes, it is a breaking change for sure, so it would require a fork or a major version increment. I guess it's up to you then if you want to keep the project alive. I'm happy to do the work. I was thinking of doing my own, but NuGet says this has 69,000+ downloads, which I would consider makes this a fairly popular package. Did you find a better alternative package?
No worries! I actually haven't used the project myself in ages (haven't had projects at work/otherwise that it really fits into), but I still try to keep it alive so long as there are people that find it useful :)
I'm totally open to PRs and as long as they make sense in the scheme and scope of the project I'm happy to include them, especially things like this that are really more matters of updating it to more modern paradigms.
Since I'm not really an active consumer of the library anymore I don't feel right about strong-arming my own modern sensibilities into the project ;) If the community is interested in those things, though, great!
Long story short, I'd be happy to look over/take a PR for this change as a new major version because I do completely agree that what you're proposing is cleaner.
I submitted a pull request. I re-did my edits because I went overboard on my first pass. I think there's more to consider before making a new release. Such as what is the point of the IoC thing? We could add an interface to make the transition a little less painless and add the extension methods back on that interface.