Clean up helper when structs with same name exist as a global struct and a cluster specific one #1562
Conversation
- Add selectStructByNameAndClusterName because selectStructByNameAndClusterId could not be used for some of our existing helpers such as asObjectiveCClass.
- Update query-util.js to form the correct data type retrieval using data type name and cluster name: sqlQueryForDataTypeByNameAndClusterName
- Update asObjectiveCClass with selectStructByNameAndClusterName instead of selectStructByName
- Github: ZAP#1559
src-electron/generator/matter/darwin/Framework/CHIP/templates/helper.js
Outdated
Show resolved
Hide resolved
39fba12 to
111b47e
Compare
src-electron/generator/matter/darwin/Framework/CHIP/templates/helper.js
Outdated
Show resolved
Hide resolved
…and a cluster specific one - Add selectStructByNameAndClusterName because selectStructByNameAndClusterId could not be used for some of our existing helpers such as asObjectiveCClass. - Update query-util.js to form the correct data type retrieval using data type name and cluster name: sqlQueryForDataTypeByNameAndClusterName - Update asObjectiveCClass with selectStructByNameAndClusterName instead of selectStructByName - Adding unit test for the above - Github: ZAP#1559
3720ad9 to
03c93d2
Compare
| let res = await dbApi | ||
| .dbAll(db, queryWithClusterName) | ||
| .then((rows) => rows.map(dbMapping.map.struct)) | ||
| if (res && res.length == 1) { |
There was a problem hiding this comment.
Why is this checking for res.length? If length is not 1 here, I would expect that's an error condition of some sort, no?
There was a problem hiding this comment.
Added that to cover the empty array returned from dbAll if not found. I can add the error conditioning for safety
There was a problem hiding this comment.
Ah, so there are three cases: length 0 (do the query without cluster), length 1 (found it), length > 1 (error)?
And "res" is never falsy? Or sometimes res is falsy and sometimes length 0 depending on ... what?
There was a problem hiding this comment.
I believe dbGet could have returned undefined/null and dbAll returns an empty array if no entries are found on the query. Based on that the cases can differ.
There was a problem hiding this comment.
Where is dbGet involved here? res is coming from dbAll, and either we get an exception or an array there, as far as I can tell, since that's what the last then clause does.
There was a problem hiding this comment.
Mentioned dbGet for just information purposes. In this case it is just dbAll. Your understanding is correct.
There was a problem hiding this comment.
Then why are we checking whether "res" is falsy? It can't be: it's an array.
There was a problem hiding this comment.
You are right. A lot of the times just end up applying defensive coding because it does not hurt having to check for nulls.
There was a problem hiding this comment.
I guess my point is that defending against things that obviously can't happen just leads to confusion as here... Anyway, approving, either way.
| let res = await dbApi | ||
| .dbAll(db, queryWithClusterName) | ||
| .then((rows) => rows.map(dbMapping.map.struct)) | ||
| if (res && res.length == 1) { |
There was a problem hiding this comment.
I guess my point is that defending against things that obviously can't happen just leads to confusion as here... Anyway, approving, either way.