Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Play nice with @types/iobroker, rename a bunch of stuff and rework the methods #4

Merged
merged 0 commits into from Nov 14, 2020

Conversation

AlCalzone
Copy link
Collaborator

@gaudes Don't be shocked - I think almost every line has changed in some way. Here's a breakdown of what I did:

Use a consequent naming scheme:

  • types start with capital letters, e.g. ObjectCommonSchema
  • JS methods are camelCase

Give things names that better describe what they do:

  • XyzSchema describes how objects look like
  • Things that affect the common part of an object have CommonAttribute in the name
  • The predefined objects are basically templates for objects, so I named them TemplateObjectDefinition
  • what you called iobObject is an object and a value, so I called it ObjectWithValue
  • makeIOBObj builds an object, so I called it buildObject
  • saveIOBObj synchronizes a desired object list with the objects DB, so I called it syncObjects

Make the definitions and predefined object templates validate themselves

I had to get a bit creative with the types here, but basically the exported objects are now passed through a function which is only used to validate the types. This lets us detect typos, whether attributes are used that are not present in the type definitions (most likely bugs in the type definitions):
grafik
or if something is even missing from those objects (like a CommonSchema for the info object type).

Use the bag-of-objects convention instead of overloads

I had to learn this the hard way while writing zwave-js, but using multiple overloads of a function with very different parameters is insanely annoying and error prone in JavaScript / TypeScript, as you can see from your attempt with ...any[]. Also you never know by looking at a function call what it does:

buildObject("bla", "blub", "abc", 1, "info", "switch", "bla");
// ???

By defining an options type, e.g.

export type BuildObjectOptions = {
	/** ID of the new object */
	id: string;
	/** Display name of the object */
	name: string;
	/** Description for the object */
	description?: string;
	/** Optional value for the corresponding state */
	value?: string | number | boolean | ioBroker.State | ioBroker.SettableState | null;
	// ^-- these are always present (if not optional)
} & (
	// v-- and only one of those combinations is allowed
	| { objectType?: undefined } // no extra options
	| {
		/** "template" tells the method to create an object from a template */
		objectType: "template",
		/** The predefined template to use for the object */
		template: ObjectTemplateNames,
	}
	| {
		/** or use the given object type */
		objectType: ObjectTypes,
		/** The role to use for the object */
		role: ObjectRoles,
	}
)

we can have a single object parameter with optional properties that can depend on the value of other properties and have TypeScript yell at us if we messed up a parameter combination. Plus you know immediately what each value stands for:

buildObject({
  id: "bla", 
  name: "blub",
  description: "abc",
  value: 1,
  objectType: "info",
  role: "switch",
  /* oops, this one is too much: */ "bla"
});

and you also don't need args[5] (what was that again?) in the implementation.

@AlCalzone
Copy link
Collaborator Author

AlCalzone commented Nov 14, 2020

One thing we need to discuss is how we want to use the role defintions. You just copied them into the common part of the object, but that can lead to this:

{
  id: ...
  common: {
    "category":[
      "info",
      "date",
    ],
    "desc":"End at date",
    "type":["string","number"],
    "read":true,
    "write":true
  }
}

which is not a valid ioBroker object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant