Return a paged DataTable

Question

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 < startRowIndex + maximumRows; i++)
 {
 if (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;
}

Answer 1

Parameter validation

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 < upperBound; i++)
 {
 if (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..).