Add functionality that allows Clinic to get the issue category from Doctor #323
Conversation
DavenportEmma
changed the title
DavenportEmma
changed the title
index.js
Outdated
| @@ -174,6 +188,18 @@ class ClinicDoctor extends events.EventEmitter { | |||
| }) | |||
| ) | |||
|
|
|||
| readStream(analysisStringified) | |||
| .then((result) => { | |||
| const issue = JSON.parse(result).issueCategory | |||
There was a problem hiding this comment.
This is kinda taking the scenic route :)
The result value was just JSON.stringify-d when the analysisStringified stream was created a few lines up, so collecting that into a string and then JSON.parsing it again is actually not necessary. We could assign this.issue in the transform() callback (now on line 186). Or, maybe a bit cleaner, we could pull the new Analysis() call into its own variable and attach an on('data') handler to it to assign the this.issue value there.
There was a problem hiding this comment.
Ah thanks, I didn't see that. That makes it much nicer
index.js
Outdated
| } | ||
|
|
||
| getIssue () { | ||
| return this.issue | ||
| } |
There was a problem hiding this comment.
Not sure this separate function is useful given that it just returns a public property. Someone could just grab this.issue directly
There was a problem hiding this comment.
Oh yeah good shout, I didn't see that. Should issue be private or do you reckon it's alright to have it as a public member?
There was a problem hiding this comment.
Oh yeah good shout, I didn't see that. Should issue be private or do you reckon it's alright to have it as a public member?
I'm running into the unexpected '#' problem, I'm going to go with leaving it as a public member
DavenportEmma
changed the title
Added an issue member that allows clinic to print the recommended next step to the console.
See clinicjs/node-clinic#234 for Clinic implementation