locked
Question about code optimization RRS feed

  • Question

  • User-384047847 posted

    Dear all,


    I have 2 code snippets what would be the bether one in performance/time of execution :

    Snippet 1 :

    public static bool IsDateTimeAWeekDay(DateTime dateTime)
    {
    // Combine all the possible DateTime calcuations
    DayOfWeek dayOfWeek = dateTime.DayOfWeek;
    int dayWeekNumber = GetWeekNumber(dateTime);

    bool isDayInDaySpan = IsDayInDaySpan(dayOfWeek.ToString());
    bool isWeekNumberInWeekDaysWeekNumber = IsWeekNumberInWeekDaysWeekNumber(dayWeekNumber);

    if (isDayInDaySpan && isWeekNumberInWeekDaysWeekNumber) return true;

    return false;
    }


    Snippet 2 :

    public static bool IsDateTimeAWeekDay2(DateTime dateTime)
    {
    // Combine all the possible DateTime calcuations
    DayOfWeek dayOfWeek = dateTime.DayOfWeek;
    int dayWeekNumber = GetWeekNumber(dateTime);

    if (IsDayInDaySpan(dayOfWeek.ToString()))
    {
    if (IsWeekNumberInWeekDaysWeekNumber(dayWeekNumber))
    {
    return true;
    }
    }

    return false;
    }


    While keeping in mind that other type of checks could be added in the future. With this I find Snippet 1 the easy to maintain than 2.

    Kind regards,

    Thursday, June 3, 2010 5:45 AM

Answers

  • User-691245060 posted

    Hello,

    Both of the code snippets perform better as you are using Short-circuit AND in first snippet and nested IF conditions in the second one...so i dont think there both differ in performance....

    But when it comes to extending code....instead of using direcly IF conditions its always better to give some names to the bool's like you did in the first snippets...that makes life easier when it comes to working with legacy code...

    So my vote goes for the 1st snippet....

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 4, 2010 2:13 AM
  • User-1910946339 posted

    I think it would be better if you could modify IsDayInDaySpan to not take a string as a parameter.  Whatever this function does I am pretty sure it would be more efficient if it used the DayOfWeek enumeration as a parameter rather than your calling code having to convert DayOfWeek to a string.  My guess is that the IsDayInDaySpan function then does some kind of test by comparing strings.  It would be much faster to compare enum values as they are basically integers.

    In your second piece of code you should move the call of GetWeekNumber inside the body of the first if statement.  If IsDayInDaySpan is false then you don't even need to calculate GetWeekNumber.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 4, 2010 3:34 AM

All replies

  • User-1630302068 posted

    Either both are O(1) runtime.  

    Thursday, June 3, 2010 5:53 PM
  • User-382039303 posted

    The second snippet is a little bit faster then first one. Because if
    IsDayInDaySpan(dayOfWeek.ToString()) is false the second one does not excute
    IsWeekNumberInWeekDaysWeekNumber function.

    Thursday, June 3, 2010 7:46 PM
  • User-691245060 posted

    Hello,

    Both of the code snippets perform better as you are using Short-circuit AND in first snippet and nested IF conditions in the second one...so i dont think there both differ in performance....

    But when it comes to extending code....instead of using direcly IF conditions its always better to give some names to the bool's like you did in the first snippets...that makes life easier when it comes to working with legacy code...

    So my vote goes for the 1st snippet....

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 4, 2010 2:13 AM
  • User-1910946339 posted

    I think it would be better if you could modify IsDayInDaySpan to not take a string as a parameter.  Whatever this function does I am pretty sure it would be more efficient if it used the DayOfWeek enumeration as a parameter rather than your calling code having to convert DayOfWeek to a string.  My guess is that the IsDayInDaySpan function then does some kind of test by comparing strings.  It would be much faster to compare enum values as they are basically integers.

    In your second piece of code you should move the call of GetWeekNumber inside the body of the first if statement.  If IsDayInDaySpan is false then you don't even need to calculate GetWeekNumber.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 4, 2010 3:34 AM
  • User-384047847 posted

    I think what ramiramilu said is what I also think.

    I can not change the way of working inside the methods, if it takes a string it takes it :) .

    I just added 2 new conditions and with the IF statements it is not easy to read, I'll keep the first version with boolean names.

    I'll thank you all for your comments.

    Friday, June 4, 2010 5:12 AM