-
Notifications
You must be signed in to change notification settings - Fork 44
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
Repeated code #21
Comments
The original idea was to have them be self-contained with no extra dependencies. I think zip archives are smart enough to contain only one copy of the same file. So it's not really a big deal I think? Any specific reason you want them to share code? |
It’d be easier to change the underlying structure of the code. If you wanted to move away from websockets for example, you totally could, if there was little repeated code. If there was a lot of repeated code, it’d be a pain.
I agree, that some things can be isolated, but always factor in the effort of changing everything. In my fork ( still wip) I had to change 21*2 files, all in exactly the same manner, to implement a twitch bot.
That is very tedious work, that could’ve been aboided. I do not see the upside of having everything as a isolated unit. The user will probably still have all Counters installes, and it just means a lot more work in the long run.
Also, the files, in the zip archive, that github creates are not nessesarily compacted. They may very well be all left as is, just all files contained in a single file.
|
I see where you're coming from, this might see a change in the future. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A lot of the socket creation and initial data collection is the same across all Counters.
Also a lot of things are repeated in many counters. Maybe it would be good to have commonly used things in a js file, which is included in all counters?
Maybe to the initial socket creation in a common file, and rely on a callback, that will be defined by each counter beforehand?
The text was updated successfully, but these errors were encountered: