ExpiryDates
has two properties PSL_ExpiryDate
and MNL_ExpiryDate
of type nullable date. I am trying to compare two objects for having same or different values.
Is there a cleaner way to do the same as the code below?
private bool SameValues(ExpiryDates ExpiryDates1, ExpiryDates ExpiryDates2)
{
//Assume they are the same value and the look for differences
bool result = true;
if (ExpiryDates1.PSL_ExpiryDate.HasValue != ExpiryDates2.PSL_ExpiryDate.HasValue)
{
result = false;
}
if (ExpiryDates1.MNL_ExpiryDate.HasValue != ExpiryDates2.MNL_ExpiryDate.HasValue)
{
result = false;
}
if ((ExpiryDates1.MNL_ExpiryDate != null) && (ExpiryDates2.MNL_ExpiryDate != null))
if (ExpiryDates1.MNL_ExpiryDate.Value != ExpiryDates2.MNL_ExpiryDate.Value)
result = false;
if ((ExpiryDates1.PSL_ExpiryDate != null) && (ExpiryDates2.PSL_ExpiryDate != null))
if (ExpiryDates1.PSL_ExpiryDate.Value != ExpiryDates2.PSL_ExpiryDate.Value)
result = false;
return result;
}
The ExpiryDate
class looks like this:
public class ExpiryDates
{
public DateTime? MNL_ExpiryDate { get; set; }
public DateTime? PSL_ExpiryDate { get; set; }
}
3 Answers 3
Nullable<T>
already does the hard work for you. Just override Equals
on your ExpiryDates
class (and GetHashCode
):
public override bool Equals(object obj)
{
var otherDates = obj as ExpiryDates;
return otherDates != null
&& MNL_ExpiryDate.Equals(otherDates.MNL_ExpiryDate)
&& PSL_ExpiryDate.Equals(otherDates.PSL_ExpiryDate);
}
As pointed out by mjolka in a comment. It would be advisible to implement IEquatable<T>
(IEquatable<ExpiryDates>
) as well. See the documentation for why it's a useful thing to do.
Further edit... It does say in the comments for IEquatable<T>
but you should additionally override ==
and !=
e.g.
public static bool operator ==(ExpiryDates left, ExpiryDates right)
{
if (object.ReferenceEquals(left, null))
{
return object.ReferenceEquals(right, null);
}
return left.Equals(right);
}
When you already know the value to return, don't guard it in a local variable. Return it immediately. In this example many conditions are evaluated even after you already know the result. If you don't return immediately, you also risk changing the local variable later in the function by mistake.
It seems you're missing some conditions. If one of ExpiryDates1.MNL_ExpiryDate
and ExpiryDates2.MNL_ExpiryDate
is null but not both, the function will return true. The same goes for the other date pairs. Or is that intended?
From phone, therfor short answer
you need to check both input parameter for null, otherwise you risk a
NullReferenceException
If
HasValue
is false then the nullable isnull
, so either check theHasValue
property or check if the nullable isnull
, but don't mix both styles.input parameter shoul be named using
camelCase
casing.using braces
{}
for single commandif
statements will help you to make your code less error prone. Also this is discussed contraversially, mixing both styles is a code smell. Be consistent in the style you use.
-
\$\begingroup\$ Just a question, why "input parameter should be named using camelCase casing" ? where do this rule come from? \$\endgroup\$jimjim– jimjim2015年07月02日 02:58:48 +00:00Commented Jul 2, 2015 at 2:58
-
1\$\begingroup\$ This comes from the NET naming guidelines. You can find a link to it in the [C#] tag wiki. \$\endgroup\$Heslacher– Heslacher2015年07月02日 03:29:42 +00:00Commented Jul 2, 2015 at 3:29
-
\$\begingroup\$ Are there any tools to go trough code and change the names to match the guidelines? This seems a task much more suited for machine than a human remembering bunch of rules cooked up by someone, something like intellisense squiggles to change the name. Doeas FX cop does this? \$\endgroup\$jimjim– jimjim2015年07月02日 05:05:54 +00:00Commented Jul 2, 2015 at 5:05
-
1\$\begingroup\$ Sorry for the delay. In the none free version of VS you have a
Code Analysis
tab in the project properties and aAnalyze
menue item to run code analysis. \$\endgroup\$Heslacher– Heslacher2015年07月02日 12:12:30 +00:00Commented Jul 2, 2015 at 12:12
Equals()
method ofExpiryDates
? \$\endgroup\$