RIAEvangelist / node-dominos-pizza-api

This is a node.js wrapper for the dominos pizza apis
MIT License
538 stars 129 forks source link

Menu option plain english #20

Closed RIAEvangelist closed 9 years ago

RIAEvangelist commented 9 years ago

Should handle menu request to create a dynamic mapping menu using the name portion of each menu object.

ie if :

xyz123 :{
    ...,
   Name : 'Awesome Sauce',
   ...
}

we should create a mapping in the menu somehow to say :

{
     'Awesome Sauce':'xyz123'
}

This will allow easier UI integration and a more intuitive code integration for developers.

madelinecameron commented 9 years ago

If I am understanding this right, this would solve the issue that I had discussed previously. Are you talking about parsing the JSON that is returned by requesting a menu from a store and then returning the Option-Name mapping instead of the huge chunk of JSON?

RIAEvangelist commented 9 years ago

Not exactly, all the data in that JSON is needed, we would just extend it with a menu items list or something.

It could even be a reference to the actual item.

pseudo :

menu.easy={};
for(i in menu.items){
    var item=menu.items[i];
    if(!item.name){
        continue;
    }
    menu.easy[
        item.name
    ]=item;
}

or something. This way it could still be referenced either way. and a simple UI list can be gotten via Object.keys(menu.easy) or whatever.

RIAEvangelist commented 9 years ago

If you want this one, make sure you assign it to you so I know not to work on it. to avoid conflicts in the repo ;)

madelinecameron commented 9 years ago

Yeah, I'll work on it.

RIAEvangelist commented 9 years ago

also, FYI you may want to install the gitter app on your phone ;)

madelinecameron commented 9 years ago

Fixed / added in 9ed05a9d85d8e1903e8f0e52d5889eee19e0e743

RIAEvangelist commented 9 years ago

What do we need lodash for?

RIAEvangelist commented 9 years ago

In pure js you can do the following :

result.result.Variants.each(
    function(value, key) {
        ...
    }
);

instead of requireing a library to do the same thing

_.forEach(
    result.result.Variants, 
    function(value, key) {
       ...
    }
)
RIAEvangelist commented 9 years ago

unless its an object...

then you do

var keys=Object.keys(result.result.Variants);
keys.each(
    function(value, key) {
        var variant=result.result.Variants[key];
        ...
    }
);
madelinecameron commented 9 years ago

It was my understanding that pure JS (currently) couldn't iterate over a JSON object which is why I used lodash rather than wrestling with JS.

But I looked at .keys and it should work. Forewarning if you implement this change, the 'code' is the key and the 'friendly name' is a value inside the object. I'll fix this so lodash isn't a requirement. It might be in one other place.

madelinecameron commented 9 years ago

Seems to work, I'll push the update before I go to bed tonight. Added a few additional tests that I want to get working.

madelinecameron commented 9 years ago

Closed again with d78182732da6243c2562ace4980860a9b02e4825

madelinecameron commented 9 years ago

Or not, I'll figure out why it didn't get committed. Switched OSes today so using a new Git client (Because I am a wimp and won't use terminal. ;D)

madelinecameron commented 9 years ago

Go look at commit 2eb61c4957a41c8d703ad6e08a406c62cf8e4480 on my personal branch. Submit a pull request if you don't care about that horribleness or I can fix it in the morning.

RIAEvangelist commented 9 years ago

Just a bit of info on JS libraries.

They are written in JS, so they dont actually extend the ability of javascript in any way, what something like lodash does is make it work the same way in different browsers, like IE and Chrome, so you don't have to write special cases for each browser.

But if a library can do it, it means it is doing it in pure js.