zen-audio-player.github.io icon indicating copy to clipboard operation
zen-audio-player.github.io copied to clipboard

Published 20 hours ago • verified publisher icon zen-audio-player

How can we refactor our code?

Open shakeelmohamed opened this issue 9 years ago • 10 comments

I made a terrible sin working on zap-cli by copy/pasting some code over to contrib.js. This is definitely not ideal, and entirely unmaintainable.

What can we do to avoid this?

I'm thinking we could break out utility functions into a new repository, then either

  1. ~~Use git submodules - will we need to use requireJS by doing so? (maybe not if our JS imports are in the right order)~~
  2. Use bower and npm within that repository (without publishing) so the code can be used on the website, CLI, & Chrome extension

shakeelmohamed avatar Jan 16 '16 10:01 shakeelmohamed

I don't understand what zap-cli is. Can you add some use case?

monicacheung avatar Jan 17 '16 04:01 monicacheung 

$ zap
Usage: zap <YouTube URL | video ID | search query>

Background: you're a developer, and live in the terminal.

  1. Somebody gives you a YouTube URL
  2. You open a terminal and type zap then paste the URL, then hit enter
  3. The URL gets parsed and http://zen-audio-player.github.io opens in your default browser as either a video lookup or a search

It might be interesting to see if we can play the video within the terminal somehow also

shakeelmohamed avatar Jan 17 '16 04:01 shakeelmohamed

I don't know about Node.js but couldn't it be done in a few lines in a shell script? Something like...

get the status code of "curl -I input or http://youtubeblahblah...input" if 200 OK, open http://zen-audio-player.github.io/?v=input else, "open http://zen-audio-player.github.io/?q=input"

Why do we need to have so much dup code?

edit :)

monicacheung avatar Jan 17 '16 07:01 monicacheung

Well, that's making an extra HTTP request. Doing it in Node gives us full cross-platform support, and it's actually not that much duplicated code but we can do better.

shakeelmohamed avatar Jan 17 '16 07:01 shakeelmohamed

If we do this, we'll need to break out the common logic - then include it via npm/bower. Submodules will be adding too much friction to the process

  • Pro: We will have really solid common code, and can write unit tests.
  • Con: Contributing to any of the 3 projects (website, Chrome extension, CLI) becomes more complicated since editing common code will involve changes to 2 repos

shakeelmohamed avatar Jan 19 '16 03:01 shakeelmohamed

Is there downside of the extra HTTP request? It can let us fail early too. Refactoring code for the website would be good. But I don't see benefit of reusing code in other projects if what you want can be requested.

monicacheung avatar Jan 19 '16 08:01 monicacheung

Is there downside of the extra HTTP request?

Not necessarily, but it's bad design and could add an unwanted amount of delay if the request takes over say 100ms. This isn't an option for the Chrome extension as it needs to be an instant operation, or it will be unusable.

shakeelmohamed avatar Jan 19 '16 23:01 shakeelmohamed

Just merged #141! Now I just have to figure out how to move the contents of zap-common.js into the zen-audio-player/zap-common repo

shakeelmohamed avatar Feb 15 '16 08:02 shakeelmohamed

@shakeelmohamed are you looking to move zap-common into a Bower module that you can then import?

caseyprovost avatar Oct 13 '17 16:10 caseyprovost

@caseyprovost originally that was the idea. It seems that the JavaScript ecosystem has shifted from bower towards webpack and just using node modules - I think we can follow that model to stay modern. Out of that process, we might go ahead and port our codebase to es6 while we're at it if there's a compile step involved in the release process

shakeelmohamed avatar Oct 16 '17 07:10 shakeelmohamed