Task/g361 network in http #362
Conversation
rg911
commented
Nov 27, 2019
rg911
commented
- Added optional para 'NetworkType' to Http to bypass Rest calls for retrieving networkType.
- Tested on NamespaceHttp.spec.ts in e2e
| if (!this.networkType) { | ||
| networkTypeResolve = observableFrom(new NodeRoutesApi(this.url).getNodeInfo()).pipe( | ||
| map(({body}) => { | ||
| return body.networkIdentifier; | ||
| }), | ||
| catchError((error) => throwError(this.errorHandling(error))), | ||
| ); |
There was a problem hiding this comment.
Every time getNetworkTypeObservable is called a new NodeRoutesApi is created and the rest endpoint is called. This is not very efficient and it may ban the server. I would move the networkTypeResolve to a read-only field and create it up in the constructor. If cached, the user has less need for hard-coding the network type in the constructor and leaves the HTTPs to handle the network type efficiently.
Java has an observable.cache method to cache the result in memory.
Once we have the factory, the cache will work much better as the cached value will be reused between multiple Http Objects (Transaction, Account, etc...).
There was a problem hiding this comment.
Can put the refactoring into a separate sdk refactor task. Also the cash would probably be better to put into the implementation e.g. client wallet etc to be able to provide more control.
4 participants
