Hexagon / node-telldus

Node bindings for telldus-core
Other
34 stars 10 forks source link

Should we "flatten" the device status? #44

Closed nilzen closed 10 years ago

nilzen commented 10 years ago

I feels a bit ugly to access the device status as it is right now.

if (device.status.status === 'ON') { ...

What do you guys think about flattening the device object? So instead of:

{ 'name': 'Foo', 'status': { 'status': 'dim', 'level': 100 }, ... }

We do:

{ 'name': 'Foo', 'status': 'dim', 'level': 100, ... }

kmpm commented 10 years ago

Like if status in an object copy all it's properties to the parent object? :-1: The nice thing with it as it is today is that we actually know that 'level' belongs to 'dim' and not something else. It's kind of self describing. That's good when you have a lot of different device types or get new ones you haven't used before. It is though, somewhat unfortunate that it's called status.status. status.name or status.description would in my opinion be better but it doesn't bother me that much.

Hexagon commented 10 years ago

I'm in favour of changing name from status.status to status.name. I don't think we should flatten it though, as it in fact is an object that can have more than one sub-object.

Edit: Maybe description is an even better alternative

nilzen commented 10 years ago

I can agree that it's nice to have a separate object which groups the state of the device, I guess its more that status.status is ugly :)

So what should it be? status.name status.description status.state

Or change status to state and have state.current?

Naming is hard! ;)

plastbox commented 10 years ago

I'm in favour of status.name. After all, the value contained therein is the name of the status, hence 'status.name'.

nilzen commented 10 years ago

I can buy that!

kmpm commented 10 years ago

:+1: For status.name