style(btc/service): replace all "void 0" to "undefined" #165
Conversation
| export function publicKeyToPayment(publicKey: string, addressType: AddressType, networkType: NetworkType) { | ||
| if (!publicKey) { | ||
| return void 0; | ||
| return undefined; |
There was a problem hiding this comment.
It should be better to throw an exception than return undefined, so that the caller can realize the problem early.
There was a problem hiding this comment.
Yes, this part of the code is not aligned with the the other functions in the address.ts. Some return undefined while some don't, I'll check them out and modify in another PR, if nessesary.
| } | ||
|
|
||
| return void 0; | ||
| return undefined; |
| } | ||
| if (status !== 200) { | ||
| return void 0 as T; | ||
| return undefined as T; |
There was a problem hiding this comment.
It is recommended to throw an exception directly here
There was a problem hiding this comment.
This part is expected but the if statement is not clear, I'll update it in another PR.
| const packedParams = params ? '?' + new URLSearchParams(pickBy(params, (val) => val !== undefined)).toString() : ''; | ||
| const withOriginHeaders = this.origin ? { origin: this.origin } : void 0; | ||
| const withAuthHeaders = requireToken && this.token ? { Authorization: `Bearer ${this.token}` } : void 0; | ||
| const withOriginHeaders = this.origin ? { origin: this.origin } : undefined; |
There was a problem hiding this comment.
| const withOriginHeaders = this.origin ? { origin: this.origin } : undefined; | |
| const withOriginHeaders = { origin: this.origin }; |
There was a problem hiding this comment.
This part is expected, avoiding transparent { origin: undeinfed } for the readers
There was a problem hiding this comment.
If the origin is undefined, the origin field will not appear in the headers for the real HTTP request and you can have a try.
There was a problem hiding this comment.
I split them for better readability, although it may have failed to achieve its job. Another solution is that these properties can also be packed into the fetch call:
fetch('xxx', {
headers: {
authorization: requireToken && this.token ? `Bearer ${this.token}` : undefined,
origin: this.origin,
...headers,
},
})
| const tx = await this.service.getBtcTransaction(txId); | ||
| if (!tx) { | ||
| return void 0; | ||
| return undefined; |
There was a problem hiding this comment.
It is recommended to throw an exception directly here
There was a problem hiding this comment.
I looked at the reference of the function and it does look weird to throw afterward when nothing was found. I'll re-think about the signature of the function, and see if something were missing.
| const vout = tx.vout[index]; | ||
| if (!vout) { | ||
| return void 0; | ||
| return undefined; |
There was a problem hiding this comment.
It is recommended to throw an exception directly here
|
This PR should be rebase from the develop branch to opt-out of the #164 code changes. |
ShookLyngs
force-pushed
the
style/remove-void-0
branch
from
a19e3a4 to
e59322e
Compare
|
After discussion, throwing errors instead of returning |
Changes
Checklist