cyclomatic complexity:
Cyclomatic complexity in code is software
metric used to indicate the complexity in the program. It is a quantitative
measure of the number is linearly independent paths through programs source
code.
Read more
about cyclomatic complexity here.
In normal terms, the cyclomatic complexity
measure tells how complex is your code. Based on the numbers given per method
in source code, one can easily tell if the code is complex or not. The
cyclomatic complexity also affects other software metrics like code
maintainability index.
The cyclomatic complexity is more in the
classes/methods where there are lot of conditional operators (e.g if..else,
while, switch statements ).
The number of lines in a class or a method
also affects the cyclomatic complexity. More number of lines means, combination
of several logic altogether, which clearly violates SRP ( single responsibility
principle).
Let’s see some examples of how cyclomatic complexity is
being calculated:
Cyclomatic complexity of 1:
public void Method()
{
Console.WriteLine("Hello
World!");
}
Cyclomatic complexity of 2:
void Method(bool condition)
{ if (condition)
{ Console.WriteLine("Hello World!"); }
}
Cyclomatic complexity of 3:
public void Method(bool condition1, bool condition2)
{ if (condition1 || condition2)
{ Console.WriteLine("Hello World!"); }
}
Cyclomatic complexity of 8:
public void Method(DayOfWeek day)
{
switch (day)
{ case DayOfWeek.Monday:
Console.WriteLine("Today is Monday!"); break;
case DayOfWeek.Tuesday:
Console.WriteLine("Today is Tuesday!"); break;
case DayOfWeek.Wednesday:
Console.WriteLine("Today is Wednesday!"); break;
case DayOfWeek.Thursday:
Console.WriteLine("Today is Thursday!"); break;
case DayOfWeek.Friday:
Console.WriteLine("Today is Friday!"); break;
case DayOfWeek.Saturday:
Console.WriteLine("Today is Saturday!"); break;
case DayOfWeek.Sunday:
Console.WriteLine("Today is Sunday!"); break;
}
}
Calculating Cyclomatic complexity using Visual Studio:
In visual studio, Go
to, Analyze option and select either the Calculate Code Metrics for Selected Projects
or Calculate Code Metrics for Solution. As the each option suggests, choosing
second option would analyze complete solution compare to previous one which
would analyze it for selected projects.
Clicking on either of
the options would open up Code Metrics Results window that would show all the
Analysis details. Result window contains analysis results in terms of Maintainability
Index, Cyclomatic complexity, Depth of Inheritance, Class coupling and Lines of
code.
A good maintainable code
would have code complexity less than 10 points. If any of your method is having
more than 10 points, then it’s surely a candidate for refactoring. Remember,
the lesser is the number the better it is maintainable.
cyclomatic complexity: what is it and how to it affects
the code
The cyclomatic complexity in code is software
metric used to indicate the complexity in the program. It is a quantitative
measure of the number is linearly independent paths through programs source
code.
Read more
about cyclomatic complexity here.
In normal terms, the cyclomatic complexity
measure tells how complex is your code. Based on the numbers given per method
in source code, one can easily tell if the code is complex or not. The
cyclomatic complexity also affects other software metrics like code
maintainability index.
The cyclomatic complexity is more in the
classes/methods where there are lot of conditional operators (e.g if..else,
while, switch statements ).
The number of lines in a class or a method
also affects the cyclomatic complexity. More number of lines means, combination
of several logic altogether, which clearly violates SRP ( single responsibility
principle).
Let’s see some examples of how cyclomatic complexity is
being calculated:
Cyclomatic complexity of 1:
public void Method()
{
Console.WriteLine("Hello
World!");
}
Cyclomatic complexity of 2:
void Method(bool condition)
{ if (condition)
{ Console.WriteLine("Hello World!"); }
}
Cyclomatic complexity of 3:
public void Method(bool condition1, bool condition2)
{ if (condition1 || condition2)
{ Console.WriteLine("Hello World!"); }
}
Cyclomatic complexity of 8:
public void Method(DayOfWeek day)
{
switch (day)
{ case DayOfWeek.Monday:
Console.WriteLine("Today is Monday!"); break;
case DayOfWeek.Tuesday:
Console.WriteLine("Today is Tuesday!"); break;
case DayOfWeek.Wednesday:
Console.WriteLine("Today is Wednesday!"); break;
case DayOfWeek.Thursday:
Console.WriteLine("Today is Thursday!"); break;
case DayOfWeek.Friday:
Console.WriteLine("Today is Friday!"); break;
case DayOfWeek.Saturday:
Console.WriteLine("Today is Saturday!"); break;
case DayOfWeek.Sunday:
Console.WriteLine("Today is Sunday!"); break;
}
}
Calculating Cyclomatic complexity using Visual Studio:
In visual studio, Go
to, Analyze option and select either the Calculate Code Metrics for Selected Projects
or Calculate Code Metrics for Solution. As the each option suggests, choosing
second option would analyze complete solution compare to previous one which
would analyze it for selected projects.
Clicking on either of
the options would open up Code Metrics Results window that would show all the
Analysis details. Result window contains analysis results in terms of Maintainability
Index, Cyclomatic complexity, Depth of Inheritance, Class coupling and Lines of
code.
A good maintainable code
would have code complexity less than 10 points. If any of your method is having
more than 10 points, then it’s surely a candidate for refactoring. Remember,
the lesser is the number the better it is maintainable.
Resolving higher Cyclomatic complexity:
The main objective of understanding
cyclomatic complexity to avoid excessive code complexity source code, so that
it clean and maintainable.
Let’s see some ways by which we can reduce
the cyclomatic complexity:
1.
Validate and remove unwanted if statements: its known problem and a
code issue that more the conditional statements, the more are the code
complexities. First validate and see which if statements can be removed so the
code complexity is reduced.
2.
Refactoring: refactoring is
helpful in dealing with very high cyclomatic complexity of source code. The
higher the number, the more it needs refactoring. Every developer should try
refactoring bigger methods into smaller methods to reduce the code complexity.
Use the single responsibility principle to break bigger classes into smaller
ones thus reducing dependency on single class doing multiple functions.
3.
Remove unnecessary/redundant else
conditions: if the if. Else block are doing some logic or returning something
based on some condition, then its better to have just one single if statement,
instead of if..else block. Remember that even the else in this case would be
redundant. See following example:
Before:
private string __GetString(OracleDataReader eventReader, string key)
{
if (__CheckDbNull(eventReader, key ) )
return "--";
else
{
return eventReader[key].ToString();
}
}
After:
private string __GetString(OracleDataReader eventReader, string key)
{
if (__CheckDbNull(eventReader, key ) )
return "--";
return eventReader[key].ToString()
}
4.
Combine conditions to reduce
cyclomatic complexity: often the simple code would have some complexity, due to
conditional statements, but rewriting those with simple LINQ conditions would
reduce the count:
Before:
List<String> filteredList = new List<String>();
foreach (String s in originalList){
if (matches(s)){
filteredList.add(s);
}
}
After:
List<String> filteredList = originalList.where(s => matches(s));
One word of caution is to
be careful with such linq queries. If you think the query can get more complex,
then it won’t serve the purpose, in that case it would be better to live with
complexity of 2, rather than 1.
We want to keep the code readable as well as simple. So make sure
your LINQ queries are not exploited in near future.
5.
Apply Strategy Pattern to get
rid of long switch cases:
One good advice I would always give to developers is to use Strategy
pattern, rather than switch…case statements. Remember that using a strategy
pattern is a replacement to your switch..case statement. Having said that, please
ensure NOT to enforce strategy for smaller switch cases, if the switch cases
are going to be extended over the period of time, then surely it’s a candidate
for strategy, otherwise, its won’t make much sense.
One good example of using strategy pattern over switch case would be
a tax calculation problem for 50 states… you know that using swich statement in
this case would be very long, so instead apply strategy pattern for achieving
low cyclomatic complexity and clean code.
private void CalculateTax(string state)
{
switch (state)
{
case "TX" :
// calculate TX TAX
return;
case "CA":
// calculate CA TAX
return;
case "AZ":
// calculate AZ TAX
return;
case "TN":
// calculate TN TAX
return;
case "SC":
// calculate SC TAX
return;
case "NC":
// calculate NC TAX
return;
case "MN":
// calculate MN TAX
return;
}
}
6.
Apply Command pattern to reduce
meat from switch…case statements:
One example from real
project: see below example of GetEventList method. Nothing wrong looks in this
method except its bit long and has cyclomatic complexity of 28.
The reason for higher
cyclomatic complexity is simple, lots of conditional statements throughout
execution.
One real world example:
Now
let’s try to refactor below code and compare the difference.
Before Refactoring:
public EventSearchResponse GetEventList(string LicenseNumber, string DisplayAll, string OrderBy, string SortColumn, int start, int PageSize, out int TotalCount, out List<Event> outEventList)
{
//LicenseNumber =
"1110065845";
EventSearchResponse
eventListResult = new EventSearchResponse();
TotalCount = 0;
List<Event> events = new List<Event>();
outEventList = events;
eventListResult.TotalCount = TotalCount;
var currentdate = DateTime.Now;
BindValues bvParam = new BindValues();
//bvParam["IN_USERTYPE"]
= userType;
bvParam["PDISPLAYALL"] = DisplayAll;
bvParam["PLICENSE_NUMBER"] = Convert.ToUInt64(LicenseNumber);
bvParam["PORDER"] = SortColumn;
bvParam["PSORTBY"] = OrderBy;
bvParam["PSTARTINGPOINT"] = start;
bvParam["PNUMOFRECORDS"] = PageSize;
bvParam["OUT_EVENT_DETAILS"] = events;
using (DbSession session =
MyDb.DefaultConnection)
{
OracleDataReader eventReader = null;
try
{
session.OpenConnection();
session.ExecuteProcedure(__SPGetEvents, bvParam);
eventReader = bvParam["OUT_EVENT_DETAILS"] as
OracleDataReader;
if (eventReader.HasRows)
{
while (eventReader.Read())
{
if (!(eventReader["EVENT_ID"] == DBNull.Value))
{
events.Add(new Event()
{
EVENT_ID = Convert.ToInt64(eventReader["EVENT_ID"] == DBNull.Value ? 0 : eventReader["EVENT_ID"]),
EVENTNAME =
eventReader["EVENTNAME"] == DBNull.Value ? "--" : eventReader["EVENTNAME"].ToString(),
EVENT_TYPE
= eventReader["EVENT_TYPE"].ToString(),
DUE_DATE =
eventReader["DUE_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["DUE_DATE"]),
EXPIRATIONDATE = eventReader["EXPIRATIONDATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["EXPIRATIONDATE"]),
EVENT_EXPIRATION_DATE = eventReader["EVENT_EXPIRATION_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["EVENT_EXPIRATION_DATE"]),
FEE = Convert.ToDecimal(eventReader["FEE"] == DBNull.Value ? null : eventReader["FEE"]),
SUBMITTED_DATE = eventReader["SUBMITTED_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["SUBMITTED_DATE"]),
EVENT_STATUS = eventReader["EVENT_STATUS"] == DBNull.Value
? "--" : eventReader["EVENT_STATUS"].ToString(),
TRACKING_METHOD_DESC = eventReader["TRACKING_METHOD_DESC"] == DBNull.Value ? "--" : eventReader["TRACKING_METHOD_DESC"].ToString(),
TRACKING_NUMBER = eventReader["TRACKING_NUMBER"] == DBNull.Value ? "--" : (eventReader["TRACKING_NUMBER"].ToString()),
TRACKING_METHOD_CD = Convert.ToInt64(eventReader["TRACKING_METHOD_CD"] == DBNull.Value ? null : eventReader["TRACKING_METHOD_CD"]),
TRACKING_METHOD_NAME = eventReader["TRACKING_METHOD_NAME"] == DBNull.Value ? "--" : eventReader["TRACKING_METHOD_NAME"].ToString(),
NOTES =
eventReader["NOTES"] == DBNull.Value ? "--" : eventReader["NOTES"].ToString(),
DUE_DATE_START = eventReader["DUE_DATE_START"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["DUE_DATE_START"]),
RENEWAL_TYPE = eventReader["RENEWAL_TYPE"].ToString(),
RENEWAL_RULE_TYPE_CD = Convert.ToInt64(eventReader["RENEWAL_TYPE"] == DBNull.Value ? null : eventReader["RENEWAL_TYPE"].ToString() == "One Time" ? "3002" : "3003"),
RECURREANCE
= eventReader["RECURRANCE"].ToString(),
RECURREANCE_PATTERN = eventReader["RECURRENCE_PATTERN"].ToString(),
MAXRECORDS
= Convert.ToInt64(eventReader["MAXRECORDS"]),
viewFlag =
eventReader["DUE_DATE"] == DBNull.Value ? "E" : DateTime.Compare(Convert.ToDateTime(Convert.ToDateTime(eventReader["DUE_DATE"]).ToString("MM/dd/yyyy")), Convert.ToDateTime(DateTime.Now.ToString("MM/dd/yyyy"))) >= 0 ? "E" : "V",
});
}
}
if (events != null)
{
if (events.Count() > 0)
{
TotalCount = Convert.ToInt32(events[0].MAXRECORDS);
eventListResult.Events = outEventList;
eventListResult.TotalCount
= TotalCount;
}
}
}
if (null != eventReader)
eventReader.Close();
session.SafeClose();
}
catch (Exception Ex)
{
ErrorLog.MyErrorLog.SafeLog(Ex);
}
finally
{
if (null != eventReader)
eventReader.Close();
session.SafeClose();
}
return eventListResult;
}
}
So, start with refactoring the most common
objects that are being used, followed by creating some private methods for
further processing.
After
refactoring:
public EventSearchResponse GetEventList(string LicenseNumber, string DisplayAll, string OrderBy, string SortColumn, int start, int PageSize, out int TotalCount, out List<Event> outEventList)
{
//LicenseNumber =
"1110065845";
EventSearchResponse eventListResult = new EventSearchResponse();
TotalCount = 0;
List<Event>
events = new List<Event>();
outEventList = events;
eventListResult.TotalCount
= TotalCount;
var currentdate = DateTime.Now;
BindValues bvParam = new BindValues();
//bvParam["IN_USERTYPE"]
= userType;
bvParam["PDISPLAYALL"] = DisplayAll;
bvParam["PLICENSE_NUMBER"] = Convert.ToUInt64(LicenseNumber);
bvParam["PORDER"] = SortColumn;
bvParam["PSORTBY"] = OrderBy;
bvParam["PSTARTINGPOINT"] = start;
bvParam["PNUMOFRECORDS"] = PageSize;
bvParam["OUT_EVENT_DETAILS"] = events;
using (DbSession session = MyDb.DefaultConnection)
{
OracleDataReader eventReader = null;
try
{
session.OpenConnection();
session.ExecuteProcedure(__SPGetEvents,
bvParam);
eventReader = bvParam["OUT_EVENT_DETAILS"] as OracleDataReader;
if (eventReader.HasRows)
{
while (eventReader.Read())
{
if (eventReader["EVENT_ID"] != DBNull.Value)
{
events.Add(__GetEventRecord(eventReader));
}
}
if (events.Any())
{
TotalCount = Convert.ToInt32(events[0].MAXRECORDS);
eventListResult.Events = outEventList;
eventListResult.TotalCount
= TotalCount;
}
}
}
catch (Exception ex)
{
ErrorLog.MyErrorLog.SafeLog(ex);
}
finally
{
if (eventReader != null) eventReader.Close();
session.SafeClose();
}
return eventListResult;
}
}
private Event __GetEventRecord( OracleDataReader eventReader)
{
return new Event()
{
EVENT_ID =
__GetInt64(eventReader, "EVENT_ID"),
EVENTNAME = __GetString(eventReader,
"EVENTNAME"),
EVENT_TYPE = eventReader["EVENT_TYPE"].ToString(),
DUE_DATE =
__GetDate(eventReader, "DUE_DATE"),
EXPIRATIONDATE =
__GetDate(eventReader, "EXPIRATIONDATE"),
EVENT_EXPIRATION_DATE =
__GetDate(eventReader, "EVENT_EXPIRATION_DATE"),
FEE =
__GetDecimal(eventReader, "FEE"),
SUBMITTED_DATE =
__GetDate(eventReader, "SUBMITTED_DATE"),
EVENT_STATUS = __GetString(eventReader,
"EVENT_STATUS"),
TRACKING_METHOD_DESC =
__GetString(eventReader, "TRACKING_METHOD_DESC"),
TRACKING_NUMBER =
__GetString(eventReader, "TRACKING_NUMBER"),
TRACKING_METHOD_CD =
__GetInt64(eventReader, "TRACKING_METHOD_CD"),
TRACKING_METHOD_NAME =
__GetString(eventReader, "TRACKING_METHOD_NAME"),
NOTES =
__GetString(eventReader, "NOTES"),
DUE_DATE_START =
__GetDate(eventReader, "DUE_DATE_START"),
RENEWAL_TYPE = eventReader["RENEWAL_TYPE"].ToString(),
RENEWAL_RULE_TYPE_CD =
__GetInt64(eventReader, "RENEWAL_TYPE_CD"),
RECURREANCE = eventReader["RECURRANCE"].ToString(),
RECURREANCE_PATTERN =
eventReader["RECURRENCE_PATTERN"].ToString(),
MAXRECORDS = Convert.ToInt64(eventReader["MAXRECORDS"]),
viewFlag =
__GetViewFlag(eventReader),
};
}
private bool __CheckDbNull(OracleDataReader reader, string key)
{
return (reader[key] == DBNull.Value);
}
private decimal __GetDecimal(OracleDataReader eventReader, string key)
{
return Convert.ToDecimal(__CheckDbNull(
eventReader, key) ? null : eventReader[key]);
}
private Int64 __GetInt64(OracleDataReader reader, string key)
{
return Convert.ToInt64(__CheckDbNull(reader, key) ? null : reader[key]);
}
private string __GetString(OracleDataReader eventReader, string key)
{
return __CheckDbNull(eventReader, key )
? "--"
: eventReader[key].ToString();
}
private DateTime? __GetDate(OracleDataReader eventReader, string key)
{
return __CheckDbNull(eventReader, key)
? null
: (DateTime?) Convert.ToDateTime(eventReader[key]);
}
private string __GetViewFlag(OracleDataReader eventReader)
{
return __CheckDbNull( eventReader, "DUE_DATE" ) ? "E"
: DateTime.Compare(
Convert.ToDateTime(Convert.ToDateTime(eventReader["DUE_DATE"]).ToString("MM/dd/yyyy")),
Convert.ToDateTime(DateTime.Now.ToString("MM/dd/yyyy")))
>= 0
? "E"
: "V";
}
After refactoring one big method into
smaller chunks, cyclomatic complexity for original method has been reduced to 8
and other private methods are well under control.
Resolving higher Cyclomatic complexity:
The main objective of understanding
cyclomatic complexity to avoid excessive code complexity source code, so that
it clean and maintainable.
Let’s see some ways by which we can reduce
the cyclomatic complexity:
1.
Validate and remove unwanted if statements: its known problem and a
code issue that more the conditional statements, the more are the code
complexities. First validate and see which if statements can be removed so the
code complexity is reduced.
2.
Refactoring: refactoring is
helpful in dealing with very high cyclomatic complexity of source code. The
higher the number, the more it needs refactoring. Every developer should try
refactoring bigger methods into smaller methods to reduce the code complexity.
Use the single responsibility principle to break bigger classes into smaller
ones thus reducing dependency on single class doing multiple functions.
3.
Remove unnecessary/redundant else
conditions: if the if. Else block are doing some logic or returning something
based on some condition, then its better to have just one single if statement,
instead of if..else block. Remember that even the else in this case would be
redundant. See following example:
Before:
private string __GetString(OracleDataReader eventReader, string key)
{
if (__CheckDbNull(eventReader, key ) )
return "--";
else
{
return eventReader[key].ToString();
}
}
After:
private string __GetString(OracleDataReader eventReader, string key)
{
if (__CheckDbNull(eventReader, key ) )
return "--";
return eventReader[key].ToString()
}
4.
Combine conditions to reduce
cyclomatic complexity: often the simple code would have some complexity, due to
conditional statements, but rewriting those with simple LINQ conditions would
reduce the count:
Before:
List<String> filteredList = new List<String>();
foreach (String s in originalList){
if (matches(s)){
filteredList.add(s);
}
}
After:
List<String> filteredList = originalList.where(s => matches(s));
One word of caution is to
be careful with such linq queries. If you think the query can get more complex,
then it won’t serve the purpose, in that case it would be better to live with
complexity of 2, rather than 1.
We want to keep the code readable as well as simple. So make sure
your LINQ queries are not exploited in near future.
5.
Apply Strategy Pattern to get
rid of long switch cases:
One good advice I would always give to developers is to use Strategy
pattern, rather than switch…case statements. Remember that using a strategy
pattern is a replacement to your switch..case statement. Having said that, please
ensure NOT to enforce strategy for smaller switch cases, if the switch cases
are going to be extended over the period of time, then surely it’s a candidate
for strategy, otherwise, its won’t make much sense.
One good example of using strategy pattern over switch case would be
a tax calculation problem for 50 states… you know that using swich statement in
this case would be very long, so instead apply strategy pattern for achieving
low cyclomatic complexity and clean code.
private void CalculateTax(string state)
{
switch (state)
{
case "TX" :
// calculate TX TAX
return;
case "CA":
// calculate CA TAX
return;
case "AZ":
// calculate AZ TAX
return;
case "TN":
// calculate TN TAX
return;
case "SC":
// calculate SC TAX
return;
case "NC":
// calculate NC TAX
return;
case "MN":
// calculate MN TAX
return;
}
}
6.
Apply Command pattern to reduce
meat from switch…case statements:
One example from real
project: see below example of GetEventList method. Nothing wrong looks in this
method except its bit long and has cyclomatic complexity of 28.
The reason for higher
cyclomatic complexity is simple, lots of conditional statements throughout
execution.
One real world example:
Now
let’s try to refactor below code and compare the difference.
Before Refactoring:
public EventSearchResponse GetEventList(string LicenseNumber, string DisplayAll, string OrderBy, string SortColumn, int start, int PageSize, out int TotalCount, out List<Event> outEventList)
{
//LicenseNumber =
"1110065845";
EventSearchResponse
eventListResult = new EventSearchResponse();
TotalCount = 0;
List<Event> events = new List<Event>();
outEventList = events;
eventListResult.TotalCount = TotalCount;
var currentdate = DateTime.Now;
BindValues bvParam = new BindValues();
//bvParam["IN_USERTYPE"]
= userType;
bvParam["PDISPLAYALL"] = DisplayAll;
bvParam["PLICENSE_NUMBER"] = Convert.ToUInt64(LicenseNumber);
bvParam["PORDER"] = SortColumn;
bvParam["PSORTBY"] = OrderBy;
bvParam["PSTARTINGPOINT"] = start;
bvParam["PNUMOFRECORDS"] = PageSize;
bvParam["OUT_EVENT_DETAILS"] = events;
using (DbSession session =
MyDb.DefaultConnection)
{
OracleDataReader eventReader = null;
try
{
session.OpenConnection();
session.ExecuteProcedure(__SPGetEvents, bvParam);
eventReader = bvParam["OUT_EVENT_DETAILS"] as
OracleDataReader;
if (eventReader.HasRows)
{
while (eventReader.Read())
{
if (!(eventReader["EVENT_ID"] == DBNull.Value))
{
events.Add(new Event()
{
EVENT_ID = Convert.ToInt64(eventReader["EVENT_ID"] == DBNull.Value ? 0 : eventReader["EVENT_ID"]),
EVENTNAME =
eventReader["EVENTNAME"] == DBNull.Value ? "--" : eventReader["EVENTNAME"].ToString(),
EVENT_TYPE
= eventReader["EVENT_TYPE"].ToString(),
DUE_DATE =
eventReader["DUE_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["DUE_DATE"]),
EXPIRATIONDATE = eventReader["EXPIRATIONDATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["EXPIRATIONDATE"]),
EVENT_EXPIRATION_DATE = eventReader["EVENT_EXPIRATION_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["EVENT_EXPIRATION_DATE"]),
FEE = Convert.ToDecimal(eventReader["FEE"] == DBNull.Value ? null : eventReader["FEE"]),
SUBMITTED_DATE = eventReader["SUBMITTED_DATE"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["SUBMITTED_DATE"]),
EVENT_STATUS = eventReader["EVENT_STATUS"] == DBNull.Value
? "--" : eventReader["EVENT_STATUS"].ToString(),
TRACKING_METHOD_DESC = eventReader["TRACKING_METHOD_DESC"] == DBNull.Value ? "--" : eventReader["TRACKING_METHOD_DESC"].ToString(),
TRACKING_NUMBER = eventReader["TRACKING_NUMBER"] == DBNull.Value ? "--" : (eventReader["TRACKING_NUMBER"].ToString()),
TRACKING_METHOD_CD = Convert.ToInt64(eventReader["TRACKING_METHOD_CD"] == DBNull.Value ? null : eventReader["TRACKING_METHOD_CD"]),
TRACKING_METHOD_NAME = eventReader["TRACKING_METHOD_NAME"] == DBNull.Value ? "--" : eventReader["TRACKING_METHOD_NAME"].ToString(),
NOTES =
eventReader["NOTES"] == DBNull.Value ? "--" : eventReader["NOTES"].ToString(),
DUE_DATE_START = eventReader["DUE_DATE_START"] == DBNull.Value ? null : (DateTime?)Convert.ToDateTime(eventReader["DUE_DATE_START"]),
RENEWAL_TYPE = eventReader["RENEWAL_TYPE"].ToString(),
RENEWAL_RULE_TYPE_CD = Convert.ToInt64(eventReader["RENEWAL_TYPE"] == DBNull.Value ? null : eventReader["RENEWAL_TYPE"].ToString() == "One Time" ? "3002" : "3003"),
RECURREANCE
= eventReader["RECURRANCE"].ToString(),
RECURREANCE_PATTERN = eventReader["RECURRENCE_PATTERN"].ToString(),
MAXRECORDS
= Convert.ToInt64(eventReader["MAXRECORDS"]),
viewFlag =
eventReader["DUE_DATE"] == DBNull.Value ? "E" : DateTime.Compare(Convert.ToDateTime(Convert.ToDateTime(eventReader["DUE_DATE"]).ToString("MM/dd/yyyy")), Convert.ToDateTime(DateTime.Now.ToString("MM/dd/yyyy"))) >= 0 ? "E" : "V",
});
}
}
if (events != null)
{
if (events.Count() > 0)
{
TotalCount = Convert.ToInt32(events[0].MAXRECORDS);
eventListResult.Events = outEventList;
eventListResult.TotalCount
= TotalCount;
}
}
}
if (null != eventReader)
eventReader.Close();
session.SafeClose();
}
catch (Exception Ex)
{
ErrorLog.MyErrorLog.SafeLog(Ex);
}
finally
{
if (null != eventReader)
eventReader.Close();
session.SafeClose();
}
return eventListResult;
}
}
So, start with refactoring the most common
objects that are being used, followed by creating some private methods for
further processing.
After
refactoring:
public EventSearchResponse GetEventList(string LicenseNumber, string DisplayAll, string OrderBy, string SortColumn, int start, int PageSize, out int TotalCount, out List<Event> outEventList)
{
//LicenseNumber =
"1110065845";
EventSearchResponse eventListResult = new EventSearchResponse();
TotalCount = 0;
List<Event>
events = new List<Event>();
outEventList = events;
eventListResult.TotalCount
= TotalCount;
var currentdate = DateTime.Now;
BindValues bvParam = new BindValues();
//bvParam["IN_USERTYPE"]
= userType;
bvParam["PDISPLAYALL"] = DisplayAll;
bvParam["PLICENSE_NUMBER"] = Convert.ToUInt64(LicenseNumber);
bvParam["PORDER"] = SortColumn;
bvParam["PSORTBY"] = OrderBy;
bvParam["PSTARTINGPOINT"] = start;
bvParam["PNUMOFRECORDS"] = PageSize;
bvParam["OUT_EVENT_DETAILS"] = events;
using (DbSession session = MyDb.DefaultConnection)
{
OracleDataReader eventReader = null;
try
{
session.OpenConnection();
session.ExecuteProcedure(__SPGetEvents,
bvParam);
eventReader = bvParam["OUT_EVENT_DETAILS"] as OracleDataReader;
if (eventReader.HasRows)
{
while (eventReader.Read())
{
if (eventReader["EVENT_ID"] != DBNull.Value)
{
events.Add(__GetEventRecord(eventReader));
}
}
if (events.Any())
{
TotalCount = Convert.ToInt32(events[0].MAXRECORDS);
eventListResult.Events = outEventList;
eventListResult.TotalCount
= TotalCount;
}
}
}
catch (Exception ex)
{
ErrorLog.MyErrorLog.SafeLog(ex);
}
finally
{
if (eventReader != null) eventReader.Close();
session.SafeClose();
}
return eventListResult;
}
}
private Event __GetEventRecord( OracleDataReader eventReader)
{
return new Event()
{
EVENT_ID =
__GetInt64(eventReader, "EVENT_ID"),
EVENTNAME = __GetString(eventReader,
"EVENTNAME"),
EVENT_TYPE = eventReader["EVENT_TYPE"].ToString(),
DUE_DATE =
__GetDate(eventReader, "DUE_DATE"),
EXPIRATIONDATE =
__GetDate(eventReader, "EXPIRATIONDATE"),
EVENT_EXPIRATION_DATE =
__GetDate(eventReader, "EVENT_EXPIRATION_DATE"),
FEE =
__GetDecimal(eventReader, "FEE"),
SUBMITTED_DATE =
__GetDate(eventReader, "SUBMITTED_DATE"),
EVENT_STATUS = __GetString(eventReader,
"EVENT_STATUS"),
TRACKING_METHOD_DESC =
__GetString(eventReader, "TRACKING_METHOD_DESC"),
TRACKING_NUMBER =
__GetString(eventReader, "TRACKING_NUMBER"),
TRACKING_METHOD_CD =
__GetInt64(eventReader, "TRACKING_METHOD_CD"),
TRACKING_METHOD_NAME =
__GetString(eventReader, "TRACKING_METHOD_NAME"),
NOTES =
__GetString(eventReader, "NOTES"),
DUE_DATE_START =
__GetDate(eventReader, "DUE_DATE_START"),
RENEWAL_TYPE = eventReader["RENEWAL_TYPE"].ToString(),
RENEWAL_RULE_TYPE_CD =
__GetInt64(eventReader, "RENEWAL_TYPE_CD"),
RECURREANCE = eventReader["RECURRANCE"].ToString(),
RECURREANCE_PATTERN =
eventReader["RECURRENCE_PATTERN"].ToString(),
MAXRECORDS = Convert.ToInt64(eventReader["MAXRECORDS"]),
viewFlag =
__GetViewFlag(eventReader),
};
}
private bool __CheckDbNull(OracleDataReader reader, string key)
{
return (reader[key] == DBNull.Value);
}
private decimal __GetDecimal(OracleDataReader eventReader, string key)
{
return Convert.ToDecimal(__CheckDbNull(
eventReader, key) ? null : eventReader[key]);
}
private Int64 __GetInt64(OracleDataReader reader, string key)
{
return Convert.ToInt64(__CheckDbNull(reader, key) ? null : reader[key]);
}
private string __GetString(OracleDataReader eventReader, string key)
{
return __CheckDbNull(eventReader, key )
? "--"
: eventReader[key].ToString();
}
private DateTime? __GetDate(OracleDataReader eventReader, string key)
{
return __CheckDbNull(eventReader, key)
? null
: (DateTime?) Convert.ToDateTime(eventReader[key]);
}
private string __GetViewFlag(OracleDataReader eventReader)
{
return __CheckDbNull( eventReader, "DUE_DATE" ) ? "E"
: DateTime.Compare(
Convert.ToDateTime(Convert.ToDateTime(eventReader["DUE_DATE"]).ToString("MM/dd/yyyy")),
Convert.ToDateTime(DateTime.Now.ToString("MM/dd/yyyy")))
>= 0
? "E"
: "V";
}
After refactoring one big method into
smaller chunks, cyclomatic complexity for original method has been reduced to 8
and other private methods are well under control.
