Please Critique my Approach for Data Import with Validation that Provides Client Status
I'm working through a CSV importing approach that I want to teach to other people I work with where I'm concerned with the following:
- Validating each record before attempting to process (e.g. persist).
- Failure on any record prevents processing.
- Providing status during validation and processing.
What I'm looking for is thoughts on if the following classes and usage example appear clear enough to meet the intent described.
There are three classes.
Importer: manages the overall algorithm, keeps track of validation errors, provides the algorithm to convert each CSV line to data construct that can be validated or processed.
Importer(CSVFile)
getProcessor(): Processor
getValidator(): Validator
getErrors(): array
isOKToProcess(): boolean
lineToData(String data): Object
Validator: validates entire file and each row to ensure it's OK to process
close(): void
currentRow(): int
hasNext(): boolean
doNext(): void
Processor: persists each row
close(): void
currentRow(): int
hasNext(): boolean
doNext(): void
As you can see the last two roughly adhere to a Java style iterator. The client interaction is thus (psuedocode - assume output is flushed immediately to the buffer):
importer = new Importer("/path/to/file.csv");
validator = importer.getValidator();
writeOutput("validating<br/>");
while (validator.hasNext()) {
validator.doNext();
writeOutput(validator.currentRow() & "<br/>");
}
validator.close();
if (!importer.isOKToProcess()) {
writeOutput("errors<br/>");
writeOutput(importer.getErrors());
return; // short circuit
}
processor = importer.getProcessor();
writeOutput("processing<br/>");
while(processor.hasNext()) {
processor.doNext();
writeOutput(processor.currentRow() & "<br/>");
}
processor.close();
Some specific questions, but please feel free to critique otherwise:
- Make sense to use two separate classes (in practice probably inner) for validating and processing? Or perhaps move开发者_如何转开发 everything into one Importer class?
getErrors()
in Validator instead, because that's where the errors are raised?currentRow()
on Importer instead of Validator and Processor because it's really the current row of the overall import?
From an OO perspective I think it is right to keep the classes seperate. I do not think it is even necessary to make the Validator
an inner class. Think of it from this angle. If you implement this generically (maybe so that one day you can pass in a grammer or some kind of specification which tells the Validator
how to validate), you can use it to validate multiple types of CSV
files. In that case, you would not want it contained in an outer Importer
class. Otherwise you would have to write an Importer
/ Validator
combination each time.
An Importer
is an importer, a Validator
is a validator.
I would definitely move the getErrors()
method into the Validator
. Otherwise its like someone else taking credit for the work you have done. Since the Validator
is doing the validation, let it tell the world of the errors it found. Don't give the glory to the Importer
, it has not done anything yet.
Your third point does not make quite sense. Both the Validator
and the Importer
has the currentRow()
method. This seems correct. They are functianally different and both need to keep track of progress. One change you might be able to make is to give the Validator
have a getTotalNumRows()
method. This way, once the file is validated, the Importer
can ask the Validator
how many rows it read during validation. This will enable the Importer
to display progress better, ie. as a percentage rather than only showing on which line it is.
Other than that, I think your design is good.
精彩评论