Conversation
…formats test implementation with @PVahldiek
…Doell/ods into adapter-nodejs-refactoring
…meric or string based enums with @PVahldiek
| @@ -0,0 +1,6 @@ | |||
| export class DataImportResponse { | |||
There was a problem hiding this comment.
If no logic => make it an interface
| @@ -0,0 +1,22 @@ | |||
| import { Format } from './enum/Format'; | |||
|
|
|||
| export class FormatConfig { | |||
There was a problem hiding this comment.
Could be pulled up to AdapterConfig => removes Java file clutter
There was a problem hiding this comment.
Probably an interface since no logic?
There was a problem hiding this comment.
What do you mean with could be pulled up? we are not sure
There was a problem hiding this comment.
I mean moving it to the AdapterConfig file since it is only a very small interface definition.
| export interface ProtocolConfig { | ||
|
|
||
| protocol:Protocol; | ||
| parameters: Map<string, any> | undefined; |
There was a problem hiding this comment.
I'd probably make it a Record<string, unknown>. Maybe makes sense to model the concrete values in the inheriting interfaces explicitly..
| import { XmlInterpreter } from "../../interpreter/XmlInterpreter"; | ||
|
|
||
|
|
||
| export class Format { |
There was a problem hiding this comment.
No class required I think... Just
export const JsonInterpreter = new JsonInterpreter();
...
Import then the following way in other files:
import * as Format from '...'
There was a problem hiding this comment.
Is beeing used as parameter in FormatConfig.ts, needs to be a class i think. Unless we dont dont need it there later on
| // To Implement | ||
| static getAllFormats(): Array<Interpreter> { | ||
| try { | ||
| // TODO implement interpreter |
There was a problem hiding this comment.
You can simplify the old very generic implementation by just listing all interpreters / importers I think...
4 participants
Don't merge, just for code feedback...