1
\$\begingroup\$

I have this code which compares two adjacent time stamps. If they are less than 15 min apart, it adds a new time stamp (row) in between with the new time stamp equal to the previous time + 15 min and value equal to the previous time stamp. If there is more than one-time stamp missing (more than 15 min interval), it adds a number of time stamps with + 15 min interval each. Sample value equal to last recorded value.

CSV file looks something like this:

"Time stamp","Value","Events","Comment","User"
"5/07/2016 7:07:54 PM","7.00","","",""
"5/07/2016 6:37:52 PM","14.00","","",""
"5/07/2016 6:22:51 PM","26.00","","",""
"5/07/2016 6:07:50 PM","9.00","","",""
"5/07/2016 7:22:06 AM","0.00","","",""
"5/07/2016 7:07:05 AM","16.00","","",""
"5/07/2016 6:52:04 AM","25.00","","",""

At the moment it is doing what it suppose to do but in wrong direction, because the .csv file was created from bottom up.

for (var currentRow = 0; currentRow < dgvValues.Rows.Count; currentRow++)
    {
        for (var cellIndex = 0; cellIndex < dgvValues.Rows[currentRow].Cells.Count; cellIndex++)
        {
            if (cellIndex > 0 && cellIndex < dgvValues.Rows[currentRow].Cells.Count)
            {
                buffer += ",";
            }
            buffer += dgvValues.Rows[currentRow].Cells[cellIndex].Value.ToString();
        }
        buffer += "\n";

        pleasewait.LabelText = (currentRow.ToString() + " of " + numRows + " rows processed");
        Application.DoEvents();

        if (currentRow + 1 >= dgvValues.Rows.Count) continue;
        {
            var firstDate = Parse(dgvValues.Rows[currentRow].Cells[0].Value.ToString());
            var secondDate = Parse(dgvValues.Rows[currentRow + 1].Cells[0].Value.ToString());

            var timespan = firstDate - secondDate;
            double min = -15;
            firstDate = firstDate.AddMinutes(min);

            if (timespan.TotalMinutes > 15)
            {
                if (firstDate == secondDate)
                    @equals = true;
                while (timespan.TotalMinutes > 15 && !@equals)
                {
                    timespan = firstDate - secondDate;
                    for (var cellIndex = 0;
                        cellIndex < dgvValues.Rows[currentRow].Cells.Count;
                        cellIndex++)
                    {
                        if (cellIndex > 0 && cellIndex < dgvValues.Rows[currentRow].Cells.Count)
                        {
                            buffer += ",";
                        }
                        if (cellIndex != 0)
                        {
                            buffer += dgvValues.Rows[currentRow].Cells[cellIndex].Value.ToString();
                        }
                        else
                            buffer += Parse(firstDate.ToString(CultureInfo.CurrentCulture));
                    }
                    buffer += "\n";
                    min = -15;
                    firstDate = firstDate.AddMinutes(min);
                }
            }
        }
    }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Do you think the offset to the rigtht is something anyone would want to deal with? \$\endgroup\$
    – paparazzo
    Commented Aug 11, 2016 at 0:38

2 Answers 2

4
\$\begingroup\$

The code is way too "linear", too deep, too hard to follow with all the indexing and generic column-cell referencing.

  • I prefer foreach over for. Simpler, cleaner, easier to read.
  • For each row, you can reference the DataGridViewCellCollection property by column name-value: theRow.Cells['theColumnName'].Value
  • Create a method that takes a DataGridViewRow parameter and simply copies its column values into a "CSVrow" object - a class you create.
    • override ToString and build the CSV string there.
  • Create a "CSVrowCollection" class to hold the CSV objects.
    • Its main purpose is to build the CSV file. Do this in a ToString override

.

class CSV {
    public CSV (DataGridViewRow timeStampRow) {  }

    public override string ToString() {
        // note the single quote characters wrapping the complete string
        return string.Format('"{0}","{1}","{2}","{3}","{4}","{5}"', 
                            Timestamp, Value, Events, Comment, User);
    }
}

class CSVcollection {
    protected List<CSV> CsvList { get; set; }

    public void Add(CSV aCsvObject) { }

    public override string ToString() {
        // building the value names string not shown.
        StringBuilder me = new StringBuilder();  //System.Text

        foreach (var record in CsvList)
           me.AppendLine(record);  //implicit ToString call

        return me.ToString();
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for this. Follow up question: How Do I replace values more than 1000 with 0 without disturbing the date "2016" \$\endgroup\$
    – QANew
    Commented Aug 11, 2016 at 3:38
  • \$\begingroup\$ I don't understand. Do you mean the potential for a year roll-over as new "+15 minute dateTimes" are created? \$\endgroup\$
    – radarbob
    Commented Aug 11, 2016 at 12:32
  • \$\begingroup\$ Re: "how do I replace ...". This could be done in the CSV constructor: this.Value = timeStampRow.Cells['Value'].Value >= 1000 ? 0 : timeStampRow.Cells['Value'].Value \$\endgroup\$
    – radarbob
    Commented Aug 12, 2016 at 17:00
1
\$\begingroup\$

@equals is really a bad name for variable. Also, if you are not using @equals else where you don't even need if condition.

Before

if (firstDate == secondDate)
    @equals = true;
while (timespan.TotalMinutes > 15 && !@equals)
{
    ...
}

After

while (timespan.TotalMinutes > 15 && firstDate != secondDate)
{
    ...
}

Use Foreach for cleaner code and RowType to check for header this will save you all kind of extra validations

foreach (GridViewRow row in dgvValues.Rows)
{
    if (row.RowType == DataControlRowType.Header)
        continue;

    ...
}

Make use of string.Format for more readability

Before

pleasewait.LabelText = (currentRow.ToString() + " of " + numRows + " rows processed");

After

pleasewait.LabelText = $"{currentRow.ToString()} of {numRows} rows processed"; // C# 6.0 

pleasewait.LabelText = string.Format("{0} of {1} rows processed", currentRow.ToString(), numRows); // Previous version of C#

You don't need braces here. This will saves you from nesting.

if (currentRow + 1 >= dgvValues.Rows.Count) continue;
{
   ...
}

Some other observations:

  • Use Environment.NewLine whenever possible.
  • You can do negative checks first and return. This will results in much readable and less number of lines in code.
  • If you are doing much string concatenation use StringBuilder class. This will give you huge performance.
  • You don't need this to access variables
  • There are lot of unnecessary conditional checks which can be avoided

After implementing above suggestions, your code will be similar to below:

var buffer = new StringBuilder();

foreach (GridViewRow row in dgvValues.Rows)
{
    if (row.RowType == DataControlRowType.Header)
        continue;

    foreach (DataGridViewCell cell in row.Cells)
    {
        buffer.Append(",");
        buffer.Append(cell.Value);
    }

    buffer.Append(Environment.NewLine);

    pleasewait.LabelText = $"{currentRow.ToString()} of {numRows} rows processed";

    ...

    if (firstDate == secondDate)
        continue;

    var timespan = firstDate - secondDate;

    while (timespan.TotalMinutes > 15)
    {
       ...
    }

}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.