patterncsharpMinor
Return a paged DataTable
Viewed 0 times
returndatatablepaged
Problem
Is there a better way to do it and have it be .Net 2.0 compatible?
public static DataTable GetErrorLog(int startRowIndex, int maximumRows, string sortExpression, string logPath)
{
if (string.IsNullOrEmpty(sortExpression))
{
sortExpression = "fileName DESC";
}
DataTable errorLog = GetErrorLogDataTable();
string[] filePaths = Directory.GetFiles(logPath);
foreach (string filePath in filePaths)
{
DataRow row = errorLog.NewRow();
row["fileName"] = Path.GetFileName(filePath);
row["filePath"] = filePath;
errorLog.Rows.Add(row);
}
DataView dataView = new DataView(errorLog);
dataView.Sort = sortExpression;
errorLog = dataView.ToTable();
DataTable pagedErrorLog = errorLog.Clone();
for (int i = startRowIndex; i = errorLog.Rows.Count)
{
break;
}
pagedErrorLog.ImportRow(errorLog.Rows[i]);
}
if (pagedErrorLog.Rows.Count <= 0)
{
return errorLog;
}
else
{
return pagedErrorLog;
}
}
private static DataTable GetErrorLogDataTable()
{
DataTable dataTable = new DataTable();
dataTable.Columns.Add("fileName");
dataTable.Columns.Add("filePath");
return dataTable;
}Solution
Parameter validation
Because
E.g assume a user of that method is passing a
A much better way would be to validate that parameter at the beginning like so
I would split that big method into several smaller ones which are having only one responsibility like
How should
Summing up the changes with only the mentioned validation which should be extended to validate the remaining parameters as well, would lead to
Other than what I had mentioned, your code looks good, you are following the naming conventions and you have named your things well (except of the
Because
GetErrorLog() is a public method you should assume that any given parameter can be wrong so a at least basic validation should take place.E.g assume a user of that method is passing a
startRowIndex == -1 then the line pagedErrorLog.ImportRow(errorLog.Rows[i]); would throw with an IndexOutOfBoundException.A much better way would be to validate that parameter at the beginning like so
if (startRowIndex < 0)
{
throw new ArgumentOutOfRangeException("startRowIndex");
}I would split that big method into several smaller ones which are having only one responsibility like
private static void FillErrorLogTable(DataTable table, string[] filePaths)
{
foreach (string filePath in filePaths)
{
DataRow row = table.NewRow();
row["fileName"] = Path.GetFileName(filePath);
row["filePath"] = filePath;
table.Rows.Add(row);
}
}
private static void FillPagedErrorLog(DataTable source, DataTable destination, int lowerBound, int upperBound)
{
int sourceRowCount = source.Rows.Count;
for (int i = lowerBound; i = sourceRowCount)
{
break;
}
destination.ImportRow(source.Rows[i]);
}
}How should
Rows.Count at this condition if (pagedErrorLog.Rows.Count <= 0) ever be < 0? That won't ever happen.Summing up the changes with only the mentioned validation which should be extended to validate the remaining parameters as well, would lead to
public static DataTable GetErrorLog(int startRowIndex, int maximumRows, string sortExpression, string logPath)
{
if (startRowIndex < 0)
{
throw new ArgumentOutOfRangeException("startRowIndex");
}
if (string.IsNullOrEmpty(sortExpression))
{
sortExpression = "fileName DESC";
}
DataTable errorLog = GetErrorLogDataTable();
string[] filePaths = Directory.GetFiles(logPath);
FillErrorLogTable(errorLog, filePaths);
DataView dataView = new DataView(errorLog);
dataView.Sort = sortExpression;
errorLog = dataView.ToTable();
DataTable pagedErrorLog = errorLog.Clone();
FillPagedErrorLog(errorLog, pagedErrorLog, startRowIndex, startRowIndex + maximumRows);
if (pagedErrorLog.Rows.Count == 0)
{
return errorLog;
}
else
{
return pagedErrorLog;
}
}Other than what I had mentioned, your code looks good, you are following the naming conventions and you have named your things well (except of the
GetErrorLogDataTable() method which I would name Create..).Code Snippets
if (startRowIndex < 0)
{
throw new ArgumentOutOfRangeException("startRowIndex");
}private static void FillErrorLogTable(DataTable table, string[] filePaths)
{
foreach (string filePath in filePaths)
{
DataRow row = table.NewRow();
row["fileName"] = Path.GetFileName(filePath);
row["filePath"] = filePath;
table.Rows.Add(row);
}
}
private static void FillPagedErrorLog(DataTable source, DataTable destination, int lowerBound, int upperBound)
{
int sourceRowCount = source.Rows.Count;
for (int i = lowerBound; i < upperBound; i++)
{
if (i >= sourceRowCount)
{
break;
}
destination.ImportRow(source.Rows[i]);
}
}public static DataTable GetErrorLog(int startRowIndex, int maximumRows, string sortExpression, string logPath)
{
if (startRowIndex < 0)
{
throw new ArgumentOutOfRangeException("startRowIndex");
}
if (string.IsNullOrEmpty(sortExpression))
{
sortExpression = "fileName DESC";
}
DataTable errorLog = GetErrorLogDataTable();
string[] filePaths = Directory.GetFiles(logPath);
FillErrorLogTable(errorLog, filePaths);
DataView dataView = new DataView(errorLog);
dataView.Sort = sortExpression;
errorLog = dataView.ToTable();
DataTable pagedErrorLog = errorLog.Clone();
FillPagedErrorLog(errorLog, pagedErrorLog, startRowIndex, startRowIndex + maximumRows);
if (pagedErrorLog.Rows.Count == 0)
{
return errorLog;
}
else
{
return pagedErrorLog;
}
}Context
StackExchange Code Review Q#136134, answer score: 2
Revisions (0)
No revisions yet.