Make "state" key optional

[koalatux]

This fixes #40

[dbrgn]

Maybe you want to use the null value to explicitly convey the information that the state is currently unknown (e.g. if the opening state determining service is temporarily offline)?

[koalatux]

Maybe I should document the difference between "no state service implemented" and "state service temporarily unavailable / unknown state"?

[rnestler]

Maybe you want to use the null value to explicitly convey the information that the state is currently unknown (e.g. if the opening state determining service is temporarily offline)?

Right.

Maybe I should document the difference between "no state service implemented" and "state service unavailable / unknown state"?

That would actually be nice, if we document how it is supposed to be useed and what it means if the state field if absent compared to if state.open is set to null.

[gidsi]

Maybe you want to use the null value to explicitly convey the information that the state is currently unknown (e.g. if the opening state determining service is temporarily offline)?

Doesn't most json librarys interpret a field set to null the same like it won't exist? Most languages don't make a difference between undefined and null so I'm not even sure if you would even be able to distinguish that.

Maybe I should document the difference between "no state service implemented" and "state service unavailable / unknown state"?

That would actually be nice, if we document how it is supposed to be useed and what it means if the state field if absent compared to if state.open is set to null.

I would vote for set the open field to not required and also for merge the change of the state field now if we disagree about the open field and separate the changes since they are not really depending on each other.

[koalatux]

Doesn't most json librarys interpret a field set to null the same like it won't exist? Most languages don't make a difference between undefined and null so I'm not even sure if you would even be able to distinguish that.

At least in Python accessing a JSON property set to null returns None and accessing an absent property gets you a KeyError.

In [1]: import json

In [2]: j = json.loads('{"a":"foo","b":null}')

In [3]: j['a']
Out[3]: 'foo'

In [4]: j['b']

In [5]: j['c']
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-5-92c0ce4ca33a> in <module>()
----> 1 j['c']

KeyError: 'c'

[koalatux]

I think making option non-nullable is worse than the status quo. Because if you want to represent the state service being unavailable, you have to omit the whole state property, with that you will lose all metadata like the icons. Furthermore it will change the structure of the data. Having a fix data structure for an endpoint makes it easier to develop a simple script to parse a specific endpoint, because you don't have to catch all corner cases of a changing data structure.

The more important thing to consider is the stability issue. Current clients might assume the state property is always present and might crash otherwise. How important is it to not introduce breaking changes in contrast to have a clean API?

Let's sum up all the possibilities:

• state required, open nullable (status quo)
  • + Does not break existing applications.
  • - Does not allow distinction between state service not implemented and state service currently unavailable.
  • - Requires a superfluous dummy object in the case where the state service is not implemented.
• state optional, open nullable (currently this PR)
  • + Can represent both cases in different ways, state service not implemented and state service currently unavailable.
  • + No empty "placeholder" objects necessary.
  • - Breaking change, because apps can't assume the state property is available.
• state optional, open not nullable
  • + No empty "placeholder" objects necessary.
  • - Does not allow distinction between state service not implemented and state service currently unavailable.
  • - Metadata of the state, e.g. the icons, is lost when the state service is currently unavailable, because the only way to represent that, is to omit the whole state property. And the resulting dynamic data structure makes developing simple scripts more error prone.
  • - Breaking change, because apps can't assume the state property is available.

[gidsi]

Oh, sorry, i had it in my head, but appreantly didn't write it down. For me removing the null type from the open field also means to remove the required flag from it.

So it would be a fourth option:

• state optional, open optional, not nullable

Which is the same as option 2 but without the null "hack".

There are approx 11 spaces right now who set it to null and for me it looks like all of them are just trying to get around the required problem.

curl https://api.spaceapi.io/collector | json_pp | grep "\"open\" : null" -C 5 

For me the breaking changes are necessary, it's an issue that software has to adapt to it but in my eyes a different problem that we should challenge on a different level (e.g. spaces can provide different versions or something similar). Releasing versions that are fully backward compatible is not applicable for me.

[koalatux]

I totally didn't think of that solution and I'm happy you are open to breaking changes :-) I will update the PR then.

[dbrgn]

Oh, one more thing: Can you update the CHANGELOG with the change (to be used as an upgrade guide)?

[dbrgn]

Thank you!