Josh
Josh

Reputation: 61

Is there a more efficient way to write IF statements that use DateTime values?

I have a hefty list of if statements which compare the current date to a set due date, it then displays how many days remain until the deadline or show if it has already been met. Just wondering if there is a more efficient way to go about this.

Below is an example of one of the 'sets' of statements, there are around 7 more of these. It's almost kind of gross, looking for a little help or a point in the right direction in how I can tidy it up. Thanks!

if (DateTime.Now >= programming1)
{
    lblReadDate.Text = "Due date for Programming Assignment 1 has been reached (" + programming1.ToShortDateString() + ").";
}
else
{
    lblReadDate.Text = "Days remaining until Programming Assignment 1 deadline: " + daysRemaining1.ToString("0");
}

if (DateTime.Now >= programming2)
{
    lblReadDate2.Text = "Due date for Programming Assignment 2 has been reached (" + programming2.ToShortDateString() + ").";
}
else
{
    lblReadDate2.Text = "Days remaining until Programming Assignment 2 deadline: " + daysRemaining2.ToString("0");
}

Upvotes: 2

Views: 191

Answers (6)

Telan Niranga
Telan Niranga

Reputation: 457

You should have class like below for you program.

    public class Program
    {
        public string Id { get; set; }
        public string Name { get; set; }
        public DateTime DueDate { get; set; }  
    }

The you can pass your program object to method like follow to get required message.

    Public string GetMessage(Program program)
    {

      if(DateTime.Now > Program.DueDate)
        return string.Format("Due date for Programming Assignment {0} has been reached {1}.",Program.Name,Program.DueDate);    
      else
        string.Format("Days remaining until Programming Assignment {0} deadline: {1}",Program.Name,Program.DueDate);

    }

If you need to check this in collection of program objects. Modify GetMessage method as required.

I hope this will help you. Happy coding. :)

Upvotes: 0

vgru
vgru

Reputation: 51284

It's hard to tell what approach will work best for you, but I am presuming you will generally want to have a class that represents an assignment, e.g.:

public class Assignment
{
    public string Name { get; set; }
    public DateTime DueDate { get; set; }

    // This is just one of many ways to do it.
    // This can be an extension method, for example.
    // Note also that DateTime.UtcNow is changing as 
    // you are traversing the list.
    public bool IsOverdue
    {
       get { return DateTime.UtcNow > DueDate; }
    }
}

General way to associate two entities (in this case, an assignment instance and a label) would be through a Dictionary<TKey, TValue>. In this case you might map by assignment name, to keep things simple:

// This is just one of many ways to do it.
// You could make Assignment implement IEquatable, or
// specify a custom comparer and map from actual
// Assignment to a Label, for instance.
var assignmentToLabelMap = new Dictionary<string, Label>
{
    { "Programming Assignment 1", label1 },
    { "Programming Assignment 2", label2 },
    { "Programming Assignment 3", label3 }
}

Once you have this in place, your UI updates are simple:

// Get the assignments
IList<Assignment> assignments = GetTheListOfAssignments();

// Update labels
foreach (var assignment in assignments)
{
    // no error checking here whatsoever
    var label = assignmentToLabelMap[assignment.Name];

    if (assignment.IsOverdue)
        label.Text = xxx;
    else
        label.Text = yyy;
}

Since you seem to be using WinForms, you might also want to implement INotifyPropertyChanged and configure control binding to make all this happen automagically, but something still needs to iterate through the list at some time to trigger the change.

Upvotes: 4

Sebastian Siemens
Sebastian Siemens

Reputation: 2421

It is easy to refactor this kind of code.

you can put the code in an extension method and just call it.

for example

lblReadDate.Text = programming1.GetAssignmentMessage("1", daysRemaining1);
lblReadDate2.Text = programming2.GetAssignmentMessage("2", daysRemaining2);

Extension Method:

public static string GetAssignmentMessage(this DateTime programming, string assignment, int daysRemaining)
{
if (DateTime.Now >= programming)
{
    return "Due date for Programming Assignment " + assignment + " has been reached (" + programming.ToShortDateString() + ").";
}
else
{
    return "Days remaining until Programming Assignment " + assignment + " deadline: " + daysRemaining.ToString("0");
}
}

Upvotes: 0

RemarkLima
RemarkLima

Reputation: 12047

You could use Ternary Operators to tidy it up visually?

lblReadDate.Text = DateTime.Now >= programming1 ?
     string.format("Due date for Programming Assignment 1 has been reached ({0})", programming1.ToShortDateString()) :
     string.format(""Days remaining until Programming Assignment 1 deadline: {0}", daysRemaining1.ToString("0"));

That said, an extension method would help further:

public static class Extensions
{
    public static bool CompareDateToNow(this DateTime compareFrom)
    {
        return DateTime.Now >= compareTo;
    }
}

Then it becomes:

lblReadDate.Text = programming1.CompareDateToNow() ?
     string.format("Due date for Programming Assignment 1 has been reached ({0})", programming1.ToShortDateString()) :
     string.format(""Days remaining until Programming Assignment 1 deadline: {0}", daysRemaining1.ToString("0"));

You could further rationalise the strings in the statements, and ultimately just pass the dates and programme name into it's own method.

Upvotes: 1

Igor
Igor

Reputation: 62258

Any time you have repeated code you should look for a way to generalize it and that usually means moving that code into a method or even a new class. Something like this would work for now. I am not using a loop because I saw you are assigning the text to a label depending on the value you are checking.

lblReadDate.Text = compareDate(programming1, "Programming Assignment 1");
lblReadDate2.Text = compareDate(programming2, "Programming Assignment 2");

// common method
public string compareDate(DateTime dateToCompare, string name) {
    if (DateTime.Now >= dateToCompare)
        return "Due date for " + name + " has been reached (" + dateToCompare.ToShortDateString() + ").";
    return "Days remaining until " + name + " deadline: " + dateToCompare.ToString("0");
}

You should also look at using string.Format which could clean up your string concanitations (it does not do anything for performance in this case, just makes it a bit easier to read).

Upvotes: 2

JPlatts
JPlatts

Reputation: 39

What about turning progamming1, progamming2, progamming3,... into a DateTime[]? You can use a for loop and keep a counter and just use counter.ToString() for the assignment number.

Upvotes: 1

Related Questions