jfroelich / rss-reader

A simple Chrome extension for viewing RSS feeds
Other
10 stars 0 forks source link

Decide whether to throw errors in patch operations or just noop #803

Closed jfroelich closed 5 years ago

jfroelich commented 5 years ago

Operations like patch-feed and patch-entry do checks against the state of the existing object, and currently, for a few property changes, throw an error if the property change should not be allowed. For example:

The thing is, maybe it is just more convenient to just noop and be more passive. Does the caller really care about knowing if a change happened or not? Stated differently, if the caller is fine with either behavior, either implementation (throw error or noop), and noop is just far simpler than error, then why not use noop?

Side note: the current implementations are not exhaustive and therefore inconsistent. e.g. deactivating inactive feed might not throw error, but activating active feed will. given the difficulty of being exhaustive, maybe the reason is just that implementation will be easier by choosing the noop design.

jfroelich commented 5 years ago

It has to do with what information the caller has. The more the caller needs to know in order to call the function without getting an error, the more burden the caller has, the more responsibility, the more annoying.

It is less annoying to noop. Should noop. This frees the caller of concern/responsibility and lets things work without getting annoying errors and requiring the caller to jump through more hoops to ensure things work. Besides, given the issues with promises and indexedDB transactions and the decision to try and abstract away indexedDB, the caller would need to do loads in a preceding separate transaction thus destroying the safety of a single transaction, thus making it very hard for the caller to really be sure they can call patch functions in this case.

jfroelich commented 5 years ago

Now it noops except for the not-found error