lanceseidman / PiCAST

PiCAST turns your $35 Raspberry Pi in to a Chromecast like Device.
https://munchron.com
1.77k stars 264 forks source link

Update goCAST.php #1

Closed xeaon closed 11 years ago

xeaon commented 11 years ago

idea, how to realize "one task per request"

saithis commented 11 years ago

How about changing the URL query string to something like: type=youtube&value=some_video_id That way only one task is possible, you still know the type and the required code would be much shorter.

xeaon commented 11 years ago

This was my first intention but i didn't want to change very much of his first version

lanceseidman commented 11 years ago

Before anything, could also do:

if ($incoming = $_GET["website"] || $incoming == $_GET["youtube"] || $incoming == "$_GET["music"]) { echo "Error"; // Process first seen request (e.g. Website)... }

This way we don't have to continue on checking for each thing if it exists, we know right away something or multiple are trying to be requested (bad on the Dev/requester for doing that but can't always win, I know). If we have a match, then grab the first match and run to that function only.

That way you can see if in the request multiple requests exist... But as I mentioned in the other Issue report, I do like the idea of the type and plan to change the Database and add it with the new addons coming.

InfernoZeus commented 11 years ago

To me, it seems like hardcoding the supported 'websites' into goCAST.php doesn't seem like a very extensible method. Ideally people should be able to easily add support for additional websites by adding one extra php file.

lanceseidman commented 11 years ago

I think maybe you're looking at a bad commit or something? No defined list of "supported" websites.

website=http://site-address-here.ext. So you can throw any website at it, no database or file of specific websites to permit/choose from and no extra PHP files needed, for anything (until music, etc come in).

I appreciate your input, thanks!

InfernoZeus commented 11 years ago

I guess website was a bad word to use for it, 'app' would probably be better. You currently have 'website', 'youtube', 'image', 'map', and 'music' hardcoded into goCAST.php, which isn't very extensible. Sure, developers can tack on additional if statements but eventually the file will grow excessively large.

lanceseidman commented 11 years ago

I have to ask, even if (right now not IF, just when) we use type, we will still have to use an if statement to determine the type and then process it.

I'm not sure where it was, maybe G+ or another ticket, a debate came up, and I am again fine with type instead of multiple columns and obviously a value column (ID | type | value) for API Calls... I mentioned we would use 1 IF to sort and find the launcher (type) exists and then use a case to process what's found.

Obviously what is in place now will never work in the future but try to remember it was the first release, it was actually for me and made changes to other files then thought to pop it on GitHub. Sadly for just me I didn't care but now with such overwhelming responses and people like you willing to help make it better. I am trying to do just that, make it better cause what you see now? Is 3 Days or so of coding, just originally to get the task done.

Keep it coming, I am listening. Your input will be implemented, I promise. So don't think I am ignoring you or putting you off as I would get that feeling too. But like I mention to everyone, give your input, debate everything, eventually someone will have the answer we all want.

Type is actually a change I think we can all agree on and so early in the development it wont hurt too bad.

saithis commented 11 years ago

With the type argument in the query string you could do something like this: Add a mysql table [ type | action ] and when a new task is received, fetch the action from the db. If there is no action for this type in the db, throw an error, otherwise execute the action.

The action could be a function name, a filename, a classname or something like that. I would prefer the classname with an autoloader and an interface for the action classes. If you intend to use 3rd party php libarys later on (for example the symfony process component for escaping the comsole commands), i would take a PSR-0 classloader (for example this one: http://symfony.com/doc/current/components/class_loader.html)

Later on you could make a plugin system, where users put their class into a plugin folder, then go to the webinterface and get a list with all plugins and a install/uninstall link. The links trigger the install/uninstall function of the class, which adds/removes the action entry to/from the db.

And even further down the road you could add a settings system for the plugins.