Doel:
Ik wil een Excel-bestand importeren en de rijen van bepaalde kolommen lezen. Hiervoor gebruik ik ExcelDataReader
. Ik “heb een low-level class geïmplementeerd genaamd ExcelData
die de ExcelDataReader
gebruikt en dingen doet zoals uitzoeken of het een” .xls “of” .xslx “-bestand (of misschien iets totaal niet-gerelateerd!) enz. Bovenop die klasse heb ik een ReadInData
-klasse gemaakt, die de specifieke kolommen die ik wil uit Excel krijgt blad.
Belangrijkste zorgen:
- De lijst met vangsten in mijn hoofdprogramma
- Gooien van uitzonderingen
- De algehele constructie van de code en codekwaliteit
- Moet ik mijn
ExcelData
klasse inkapselen inReadInData
, of moet ik het behouden zoals het is ? - Doorgeven van de
isFirstRowAsColumnNames
parameter
Omdat dit code is voor een bedrijf, heb ik de namen van een paar gewijzigd van klassen, dus ik weet dat ze “niet de beste namen zijn.
Het beginpunt van mijn code:
class Program { static void Main(string[] args) { try { ReadInData readInData = new ReadInData(@"C:\SC.xlsx", "sc_2014"); IEnumerable<Recipient> recipients = readInData.GetData(); } catch (FileNotFoundException ex) { Console.WriteLine(ex.Message); } catch (ArgumentException ex) { Console.WriteLine(ex.Message); } catch (WorksheetDoesNotExistException ex) { Console.WriteLine(ex.Message); } catch (FileToBeProcessedIsNotInTheCorrectFormatException ex) { Console.WriteLine(ex.Message); } Console.ReadLine(); } }
In deze code Ik maak een nieuwe ReadInData
klasse, die ik passeer s het pad, de naam van het bestand en de naam van het Excel-blad dat ik wil lezen.
Zorg hier: is het oké om die dingen in dat bestand door te geven?
De ReadInData
class:
public class ReadInData { private string path; private string worksheetName; public ReadInData(string path, string worksheetName) { this.path = path; this.worksheetName = worksheetName; } public IEnumerable<Recipient> GetData(bool isFirstRowAsColumnNames = true) { var excelData = new ExcelData(path); var dataRows = excelData.GetData(worksheetName, isFirstRowAsColumnNames); return dataRows.Select(dataRow => new Recipient() { Municipality = dataRow["Municipality"].ToString(), Sexe = dataRow["Sexe"].ToString(), LivingArea = dataRow["LivingArea"].ToString() }).ToList(); } }
Eigenlijk dacht ik dat ik een class nodig had bovenop de ExcelData
class, omdat het een wat hoger niveau leek.
De ExcelData
class:
public class ExcelData { private string path; public ExcelData(string path) { this.path = path; } private IExcelDataReader GetExcelDataReader(bool isFirstRowAsColumnNames) { using (FileStream fileStream = File.Open(path, FileMode.Open, FileAccess.Read)) { IExcelDataReader dataReader; if (path.EndsWith(".xls")) { dataReader = ExcelReaderFactory.CreateBinaryReader(fileStream); } else if (path.EndsWith(".xlsx")) { dataReader = ExcelReaderFactory.CreateOpenXmlReader(fileStream); } else { //Throw exception for things you cannot correct throw new FileToBeProcessedIsNotInTheCorrectFormatException("The file to be processed is not an Excel file"); } dataReader.IsFirstRowAsColumnNames = isFirstRowAsColumnNames; return dataReader; } } private DataSet GetExcelDataAsDataSet(bool isFirstRowAsColumnNames) { return GetExcelDataReader(isFirstRowAsColumnNames).AsDataSet(); } private DataTable GetExcelWorkSheet(string workSheetName, bool isFirstRowAsColumnNames) { DataSet dataSet = GetExcelDataAsDataSet(isFirstRowAsColumnNames); DataTable workSheet = dataSet.Tables[workSheetName]; if (workSheet == null) { throw new WorksheetDoesNotExistException(string.Format("The worksheet {0} does not exist, has an incorrect name, or does not have any data in the worksheet", workSheetName)); } return workSheet; } public IEnumerable<DataRow> GetData(string workSheetName, bool isFirstRowAsColumnNames = true) { DataTable workSheet = GetExcelWorkSheet(workSheetName, isFirstRowAsColumnNames); IEnumerable<DataRow> rows = from DataRow row in workSheet.Rows select row; return rows; } }
En tot slot, de Recipient
klasse (die niets bijzonders doet):
namespace ConsoleApplicationForTestingPurposes { public class Recipient { public string Municipality { get; set; } public string Sexe { get; set; } public string LivingArea { get; set; } } }
De uitzonderingsklassen erven van Exception
, en geven gewoon een bericht door aan de uitzondering.
Reacties
Antwoord
Vereenvoudig uw vangketen
static void Main(string[] args) { try { ReadInData readInData = new ReadInData(@"C:\SC.xlsx", "sc_2014"); IEnumerable<Recipient> recipients = readInData.GetData(); } catch (Exception ex) { if(!(ex is FileNotFoundException || ex is ArgumentException || ex is FileToBeProcessedIsNotInTheCorrectFormatException)) throw; Console.WriteLine(ex.Message); } Console.Write(Press any key to continue...); Console.ReadKey(true); }
Ik zie geen reden om ReadInData
een niet-statische klasse te hebben. U profiteert niet van het feit dat u het pad en de naam van het werkblad onthoudt, en u houdt geen open verbinding met de werkmap. En laat uw code er altijd complexer uitzien door variabelen te verwijderen die alleen één keer worden gebruikt (optioneel). Er is geen reden om dit ToList()
te gebruiken, omdat je toch een IEnumerable<T>
retourneert.
public static class ReadInData { public static IEnumerable<Recipient> GetData(string path, string worksheetName, bool isFirstRowAsColumnNames = true) { return new ExcelData(path).GetData(worksheetName, isFirstRowAsColumnNames) .Select(dataRow => new Recipient() { Municipality = dataRow["Municipality"].ToString(), Sexe = dataRow["Sexe"].ToString(), LivingArea = dataRow["LivingArea"].ToString() }); } }
Je kunt hetzelfde doen voor je ExcelData
klasse, dat wil zeggen .. deze functies maken in plaats van methoden op een klas. Dat hoeft niet, maar nogmaals, je profiteert niet veel van de OO, dus je moet het OO halen.
Als je dat niet doet wil / heeft iemand je ExcelData
klasse nodig, dan kapselt het in. Ik denk echter dat je deze Excel-lezer op een gegeven moment opnieuw wilt gebruiken, in welk geval ik deze methoden naar een statische ExcelHelperClass
zou verplaatsen. En je eerste klas ReadInData
Ik zou gewoon een methode maken in je originele programma.
private static IExcelDataReader GetExcelDataReader(string path, bool isFirstRowAsColumnNames) { using (FileStream fileStream = File.Open(path, FileMode.Open, FileAccess.Read)) { IExcelDataReader dataReader; if (path.EndsWith(".xls")) dataReader = ExcelReaderFactory.CreateBinaryReader(fileStream); else if (path.EndsWith(".xlsx")) dataReader = ExcelReaderFactory.CreateOpenXmlReader(fileStream); else throw new FileToBeProcessedIsNotInTheCorrectFormatException("The file to be processed is not an Excel file"); dataReader.IsFirstRowAsColumnNames = isFirstRowAsColumnNames; return dataReader; } } private static DataSet GetExcelDataAsDataSet(string path, bool isFirstRowAsColumnNames) { return GetExcelDataReader(path, isFirstRowAsColumnNames).AsDataSet(); } private static DataTable GetExcelWorkSheet(string path, string workSheetName, bool isFirstRowAsColumnNames) { DataTable workSheet = GetExcelDataAsDataSet(path, isFirstRowAsColumnNames).Tables[workSheetName]; if (workSheet == null) throw new WorksheetDoesNotExistException(string.Format("The worksheet {0} does not exist, has an incorrect name, or does not have any data in the worksheet", workSheetName)); return workSheet; } private static IEnumerable<DataRow> GetData(string path, string workSheetName, bool isFirstRowAsColumnNames = true) { return from DataRow row in GetExcelWorkSheet(path, workSheetName, isFirstRowAsColumnNames).Rows select row; }
Zoals vermeld in de commentaren, houdt u niet rekening met alle Excel-bestandstypen.
Als het niet “voor de bool isFirstRowAsColumnNames
was, zou ik u aanraden om echt verder te gaan met het hele OOP-ding, en laat de werkmap laden wanneer je het construeert, en profiteer van het OO-ontwerp door feitelijk interne gegevens te hebben naast alleen het pad. Ik denk dat het prima is dat je ze de optie geeft om isFirstRowAsColumnNames
, dat zou erg handig kunnen zijn.
Opmerkingen
- Allereerst hartelijk dank voor uw tijd om samen wat er verbeterd kan worden aan mijn code. Ik heb echter een paar vragen; de eerste is dat ik ‘ niet zeker weet wat je bedoelde met ” En je eerste klas ReadInData Ik zou gewoon een methode maken in je originele programma. Het tweede is, dat ik ‘ ook niet echt zeker weet wat je bedoelde met de eerste zin van je laatste alinea.Bedoel je dat als ik niet ‘ de parameter isFirstRowAsColumnNames zou hebben, ik mijn code min of meer zou kunnen behouden zoals hij was? Zo ja, waarom zou het daarvan afhangen?
".xlsm"
en".xlsb"
, evenals de sjabloontypen …