Calculate date for ordinal day of week in month

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












I am writing some date utility methods for a project.
One of the methods will answer the following questions (for example):



'When is the (3rd) (Monday) of (February) in the year (2018)'
'When is the (1st) (Wednesday) of (October) in the year (2020)'


The method accepts the 4 int value parameters as arguments, and will return a Date.



Here is the method as written:



/**
* Example usage:
* find the 3rd Monday of February, in the year 2018 getDayOfMonth(3, Calendar.MONDAY, Calendar.FEBRUARY, 2018)
* find the 1st Tuesday of October, in the year 2017 getDayOfMOnth(1, Calendar.TUESDAY, Calendar.OCTOBER, 2017)
* @param n occurrence count
* @param dayOfWeek day of week to find
* @param month month to use
* @param year year to use
* @return Date that the nth dayOfWeek occurs
*/
public static Date getDate(int n, int dayOfWeek, int month, int year)
Calendar calendar = Calendar.getInstance();
calendar.set(Calendar.YEAR, year);
calendar.set(Calendar.MONTH, month);
calendar.set(Calendar.DAY_OF_MONTH, 1);

int matchCount = 0;

while(true)
if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
matchCount++;

if(matchCount == n)
break;

calendar.add(Calendar.DATE, 1);
if(calendar.get(Calendar.MONTH) != month)
break;



if(matchCount != n)
throw new RuntimeException("error");


return calendar.getTime();



I am struggling to come up with a good name for the method and the 1st parameter.







share|improve this question



























    up vote
    3
    down vote

    favorite












    I am writing some date utility methods for a project.
    One of the methods will answer the following questions (for example):



    'When is the (3rd) (Monday) of (February) in the year (2018)'
    'When is the (1st) (Wednesday) of (October) in the year (2020)'


    The method accepts the 4 int value parameters as arguments, and will return a Date.



    Here is the method as written:



    /**
    * Example usage:
    * find the 3rd Monday of February, in the year 2018 getDayOfMonth(3, Calendar.MONDAY, Calendar.FEBRUARY, 2018)
    * find the 1st Tuesday of October, in the year 2017 getDayOfMOnth(1, Calendar.TUESDAY, Calendar.OCTOBER, 2017)
    * @param n occurrence count
    * @param dayOfWeek day of week to find
    * @param month month to use
    * @param year year to use
    * @return Date that the nth dayOfWeek occurs
    */
    public static Date getDate(int n, int dayOfWeek, int month, int year)
    Calendar calendar = Calendar.getInstance();
    calendar.set(Calendar.YEAR, year);
    calendar.set(Calendar.MONTH, month);
    calendar.set(Calendar.DAY_OF_MONTH, 1);

    int matchCount = 0;

    while(true)
    if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
    matchCount++;

    if(matchCount == n)
    break;

    calendar.add(Calendar.DATE, 1);
    if(calendar.get(Calendar.MONTH) != month)
    break;



    if(matchCount != n)
    throw new RuntimeException("error");


    return calendar.getTime();



    I am struggling to come up with a good name for the method and the 1st parameter.







    share|improve this question























      up vote
      3
      down vote

      favorite









      up vote
      3
      down vote

      favorite











      I am writing some date utility methods for a project.
      One of the methods will answer the following questions (for example):



      'When is the (3rd) (Monday) of (February) in the year (2018)'
      'When is the (1st) (Wednesday) of (October) in the year (2020)'


      The method accepts the 4 int value parameters as arguments, and will return a Date.



      Here is the method as written:



      /**
      * Example usage:
      * find the 3rd Monday of February, in the year 2018 getDayOfMonth(3, Calendar.MONDAY, Calendar.FEBRUARY, 2018)
      * find the 1st Tuesday of October, in the year 2017 getDayOfMOnth(1, Calendar.TUESDAY, Calendar.OCTOBER, 2017)
      * @param n occurrence count
      * @param dayOfWeek day of week to find
      * @param month month to use
      * @param year year to use
      * @return Date that the nth dayOfWeek occurs
      */
      public static Date getDate(int n, int dayOfWeek, int month, int year)
      Calendar calendar = Calendar.getInstance();
      calendar.set(Calendar.YEAR, year);
      calendar.set(Calendar.MONTH, month);
      calendar.set(Calendar.DAY_OF_MONTH, 1);

      int matchCount = 0;

      while(true)
      if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
      matchCount++;

      if(matchCount == n)
      break;

      calendar.add(Calendar.DATE, 1);
      if(calendar.get(Calendar.MONTH) != month)
      break;



      if(matchCount != n)
      throw new RuntimeException("error");


      return calendar.getTime();



      I am struggling to come up with a good name for the method and the 1st parameter.







      share|improve this question













      I am writing some date utility methods for a project.
      One of the methods will answer the following questions (for example):



      'When is the (3rd) (Monday) of (February) in the year (2018)'
      'When is the (1st) (Wednesday) of (October) in the year (2020)'


      The method accepts the 4 int value parameters as arguments, and will return a Date.



      Here is the method as written:



      /**
      * Example usage:
      * find the 3rd Monday of February, in the year 2018 getDayOfMonth(3, Calendar.MONDAY, Calendar.FEBRUARY, 2018)
      * find the 1st Tuesday of October, in the year 2017 getDayOfMOnth(1, Calendar.TUESDAY, Calendar.OCTOBER, 2017)
      * @param n occurrence count
      * @param dayOfWeek day of week to find
      * @param month month to use
      * @param year year to use
      * @return Date that the nth dayOfWeek occurs
      */
      public static Date getDate(int n, int dayOfWeek, int month, int year)
      Calendar calendar = Calendar.getInstance();
      calendar.set(Calendar.YEAR, year);
      calendar.set(Calendar.MONTH, month);
      calendar.set(Calendar.DAY_OF_MONTH, 1);

      int matchCount = 0;

      while(true)
      if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
      matchCount++;

      if(matchCount == n)
      break;

      calendar.add(Calendar.DATE, 1);
      if(calendar.get(Calendar.MONTH) != month)
      break;



      if(matchCount != n)
      throw new RuntimeException("error");


      return calendar.getTime();



      I am struggling to come up with a good name for the method and the 1st parameter.









      share|improve this question












      share|improve this question




      share|improve this question








      edited Feb 25 at 5:26









      200_success

      123k14142399




      123k14142399









      asked Feb 25 at 2:48









      user1154644

      18415




      18415




















          3 Answers
          3






          active

          oldest

          votes

















          up vote
          2
          down vote













          For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.



          The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:



          Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.



          If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.






          share|improve this answer




























            up vote
            1
            down vote













            In addition to @MyStackRunnethOver notes, here is what I think you can improve:



            Parameters validation



            First of all, the function does not check the validity of any of its input. This may raise bugs in your application. A practical case is when the user types 1 to refer to January and ends up by swimming in February because the first month of the year is JANUARY which is 0.




            while() loop optimization



            In case we set n to be 29, for instance, the while() loop will iterate that much. You can easily optimize that by early checking that n is within a set of (1, 2, 3, 4, 5) because 5 is the maximum number of times a day can re-appear within the same month.



            Think of persistence



            In case one day you want to use this utility to save dates into a database, then may be you should deal with the Gregorian calendar (Calendar calendar = GregorianCalendar.getInstance();) instead as it is the commonly used by SGBDR.






            share|improve this answer




























              up vote
              0
              down vote













              Less iteration




              public static Date getDate(int n, int dayOfWeek, int month, int year)
              Calendar calendar = Calendar.getInstance();
              calendar.set(Calendar.YEAR, year);
              calendar.set(Calendar.MONTH, month);
              calendar.set(Calendar.DAY_OF_MONTH, 1);

              int matchCount = 0;

              while(true)
              if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
              matchCount++;

              if(matchCount == n)
              break;

              calendar.add(Calendar.DATE, 1);
              if(calendar.get(Calendar.MONTH) != month)
              break;



              if(matchCount != n)
              throw new RuntimeException("error");


              return calendar.getTime();




              Consider



              public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) ordinal <= 0) 
              throw new RuntimeException("error");


              return calendar.getTime();



              To me, a method named get returns a field from the object or class. This doesn't. Instead, it calculates a date for a particular input. I would find calculateDateFor sufficient, but you can write out something like calculateDateForOrdinalDayOfWeekInMonthYear if you prefer.



              The nth something is the ordinal number or ordinal for short. You may of course prefer ordinalNumber.



              In your original, you iterate by one each time. However, once on the right day of the week, you can iterate by seven.



              The name matchCount doesn't indicate what is being matched. So I changed it to weekCount instead.



              You check, after the fact, that the ordinal value was not too large. But you don't check that it isn't too small. I added that check to the exception gate. The original behavior was to print the first day of the month whenever the ordinal value is less than or equal to zero. This version changes that to be an exception instead.



              With math



              You use loops here, but that's not necessary. It's possible to get the same effect with just math.



              public static final int DAY_COUNT_PER_WEEK = 7;

              public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year)
              Calendar calendar = Calendar.getInstance();
              calendar.set(Calendar.YEAR, year);
              calendar.set(Calendar.MONTH, month);
              calendar.set(Calendar.DAY_OF_MONTH, 1);

              int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);

              // if the day of the week sought is before the day of the week
              // of the first day of the month, we need to add a week
              if (dayCountTo < 0)
              dayCountTo += DAY_COUNT_PER_WEEK;


              dayCountTo += DAY_COUNT_PER_WEEK * (ordinal - 1);

              calendar.add(Calendar.DATE, dayCountTo);

              if (calendar.get(Calendar.MONTH) != month)
              throw new RuntimeException("error");


              return calendar.getTime();



              Note that now if the ordinal number is too small or too large, it will produce an invalid month and trigger the exception. So we can remove the explicit check on ordinal.



              Alternately, consider



              public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) 
              Calendar calendar = Calendar.getInstance();
              calendar.set(Calendar.YEAR, year);
              calendar.set(Calendar.MONTH, month);
              calendar.set(Calendar.DAY_OF_MONTH, 1);

              int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);
              calendar.add(Calendar.DATE, dayCountTo);

              // if the day of the week sought is before the day of the week
              // of the first day of the month, we need to add a week
              // but ordinal is not zero-indexed, so we have to subtract a week
              // net result is to sometimes use ordinal and otherwise ordinal - 1
              int weekCountTo = (dayCountTo < 0) ? ordinal : (ordinal - 1);

              calendar.add(Calendar.WEEK_OF_MONTH, weekCountTo);

              if (calendar.get(Calendar.MONTH) != month)
              throw new RuntimeException("error3");


              return calendar.getTime();



              This avoids manually setting the number of days in the week and relies on Calendar to perform the math.






              share|improve this answer





















                Your Answer




                StackExchange.ifUsing("editor", function ()
                return StackExchange.using("mathjaxEditing", function ()
                StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
                StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
                );
                );
                , "mathjax-editing");

                StackExchange.ifUsing("editor", function ()
                StackExchange.using("externalEditor", function ()
                StackExchange.using("snippets", function ()
                StackExchange.snippets.init();
                );
                );
                , "code-snippets");

                StackExchange.ready(function()
                var channelOptions =
                tags: "".split(" "),
                id: "196"
                ;
                initTagRenderer("".split(" "), "".split(" "), channelOptions);

                StackExchange.using("externalEditor", function()
                // Have to fire editor after snippets, if snippets enabled
                if (StackExchange.settings.snippets.snippetsEnabled)
                StackExchange.using("snippets", function()
                createEditor();
                );

                else
                createEditor();

                );

                function createEditor()
                StackExchange.prepareEditor(
                heartbeatType: 'answer',
                convertImagesToLinks: false,
                noModals: false,
                showLowRepImageUploadWarning: true,
                reputationToPostImages: null,
                bindNavPrevention: true,
                postfix: "",
                onDemand: true,
                discardSelector: ".discard-answer"
                ,immediatelyShowMarkdownHelp:true
                );



                );








                 

                draft saved


                draft discarded


















                StackExchange.ready(
                function ()
                StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188301%2fcalculate-date-for-ordinal-day-of-week-in-month%23new-answer', 'question_page');

                );

                Post as a guest






























                3 Answers
                3






                active

                oldest

                votes








                3 Answers
                3






                active

                oldest

                votes









                active

                oldest

                votes






                active

                oldest

                votes








                up vote
                2
                down vote













                For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.



                The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:



                Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.



                If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.






                share|improve this answer

























                  up vote
                  2
                  down vote













                  For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.



                  The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:



                  Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.



                  If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.






                  share|improve this answer























                    up vote
                    2
                    down vote










                    up vote
                    2
                    down vote









                    For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.



                    The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:



                    Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.



                    If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.






                    share|improve this answer













                    For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.



                    The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:



                    Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.



                    If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.







                    share|improve this answer













                    share|improve this answer



                    share|improve this answer











                    answered Feb 25 at 5:52









                    MyStackRunnethOver

                    35217




                    35217






















                        up vote
                        1
                        down vote













                        In addition to @MyStackRunnethOver notes, here is what I think you can improve:



                        Parameters validation



                        First of all, the function does not check the validity of any of its input. This may raise bugs in your application. A practical case is when the user types 1 to refer to January and ends up by swimming in February because the first month of the year is JANUARY which is 0.




                        while() loop optimization



                        In case we set n to be 29, for instance, the while() loop will iterate that much. You can easily optimize that by early checking that n is within a set of (1, 2, 3, 4, 5) because 5 is the maximum number of times a day can re-appear within the same month.



                        Think of persistence



                        In case one day you want to use this utility to save dates into a database, then may be you should deal with the Gregorian calendar (Calendar calendar = GregorianCalendar.getInstance();) instead as it is the commonly used by SGBDR.






                        share|improve this answer

























                          up vote
                          1
                          down vote













                          In addition to @MyStackRunnethOver notes, here is what I think you can improve:



                          Parameters validation



                          First of all, the function does not check the validity of any of its input. This may raise bugs in your application. A practical case is when the user types 1 to refer to January and ends up by swimming in February because the first month of the year is JANUARY which is 0.




                          while() loop optimization



                          In case we set n to be 29, for instance, the while() loop will iterate that much. You can easily optimize that by early checking that n is within a set of (1, 2, 3, 4, 5) because 5 is the maximum number of times a day can re-appear within the same month.



                          Think of persistence



                          In case one day you want to use this utility to save dates into a database, then may be you should deal with the Gregorian calendar (Calendar calendar = GregorianCalendar.getInstance();) instead as it is the commonly used by SGBDR.






                          share|improve this answer























                            up vote
                            1
                            down vote










                            up vote
                            1
                            down vote









                            In addition to @MyStackRunnethOver notes, here is what I think you can improve:



                            Parameters validation



                            First of all, the function does not check the validity of any of its input. This may raise bugs in your application. A practical case is when the user types 1 to refer to January and ends up by swimming in February because the first month of the year is JANUARY which is 0.




                            while() loop optimization



                            In case we set n to be 29, for instance, the while() loop will iterate that much. You can easily optimize that by early checking that n is within a set of (1, 2, 3, 4, 5) because 5 is the maximum number of times a day can re-appear within the same month.



                            Think of persistence



                            In case one day you want to use this utility to save dates into a database, then may be you should deal with the Gregorian calendar (Calendar calendar = GregorianCalendar.getInstance();) instead as it is the commonly used by SGBDR.






                            share|improve this answer













                            In addition to @MyStackRunnethOver notes, here is what I think you can improve:



                            Parameters validation



                            First of all, the function does not check the validity of any of its input. This may raise bugs in your application. A practical case is when the user types 1 to refer to January and ends up by swimming in February because the first month of the year is JANUARY which is 0.




                            while() loop optimization



                            In case we set n to be 29, for instance, the while() loop will iterate that much. You can easily optimize that by early checking that n is within a set of (1, 2, 3, 4, 5) because 5 is the maximum number of times a day can re-appear within the same month.



                            Think of persistence



                            In case one day you want to use this utility to save dates into a database, then may be you should deal with the Gregorian calendar (Calendar calendar = GregorianCalendar.getInstance();) instead as it is the commonly used by SGBDR.







                            share|improve this answer













                            share|improve this answer



                            share|improve this answer











                            answered Feb 25 at 7:42









                            Billal BEGUERADJ

                            1




                            1




















                                up vote
                                0
                                down vote













                                Less iteration




                                public static Date getDate(int n, int dayOfWeek, int month, int year)
                                Calendar calendar = Calendar.getInstance();
                                calendar.set(Calendar.YEAR, year);
                                calendar.set(Calendar.MONTH, month);
                                calendar.set(Calendar.DAY_OF_MONTH, 1);

                                int matchCount = 0;

                                while(true)
                                if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
                                matchCount++;

                                if(matchCount == n)
                                break;

                                calendar.add(Calendar.DATE, 1);
                                if(calendar.get(Calendar.MONTH) != month)
                                break;



                                if(matchCount != n)
                                throw new RuntimeException("error");


                                return calendar.getTime();




                                Consider



                                public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) ordinal <= 0) 
                                throw new RuntimeException("error");


                                return calendar.getTime();



                                To me, a method named get returns a field from the object or class. This doesn't. Instead, it calculates a date for a particular input. I would find calculateDateFor sufficient, but you can write out something like calculateDateForOrdinalDayOfWeekInMonthYear if you prefer.



                                The nth something is the ordinal number or ordinal for short. You may of course prefer ordinalNumber.



                                In your original, you iterate by one each time. However, once on the right day of the week, you can iterate by seven.



                                The name matchCount doesn't indicate what is being matched. So I changed it to weekCount instead.



                                You check, after the fact, that the ordinal value was not too large. But you don't check that it isn't too small. I added that check to the exception gate. The original behavior was to print the first day of the month whenever the ordinal value is less than or equal to zero. This version changes that to be an exception instead.



                                With math



                                You use loops here, but that's not necessary. It's possible to get the same effect with just math.



                                public static final int DAY_COUNT_PER_WEEK = 7;

                                public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year)
                                Calendar calendar = Calendar.getInstance();
                                calendar.set(Calendar.YEAR, year);
                                calendar.set(Calendar.MONTH, month);
                                calendar.set(Calendar.DAY_OF_MONTH, 1);

                                int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);

                                // if the day of the week sought is before the day of the week
                                // of the first day of the month, we need to add a week
                                if (dayCountTo < 0)
                                dayCountTo += DAY_COUNT_PER_WEEK;


                                dayCountTo += DAY_COUNT_PER_WEEK * (ordinal - 1);

                                calendar.add(Calendar.DATE, dayCountTo);

                                if (calendar.get(Calendar.MONTH) != month)
                                throw new RuntimeException("error");


                                return calendar.getTime();



                                Note that now if the ordinal number is too small or too large, it will produce an invalid month and trigger the exception. So we can remove the explicit check on ordinal.



                                Alternately, consider



                                public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) 
                                Calendar calendar = Calendar.getInstance();
                                calendar.set(Calendar.YEAR, year);
                                calendar.set(Calendar.MONTH, month);
                                calendar.set(Calendar.DAY_OF_MONTH, 1);

                                int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);
                                calendar.add(Calendar.DATE, dayCountTo);

                                // if the day of the week sought is before the day of the week
                                // of the first day of the month, we need to add a week
                                // but ordinal is not zero-indexed, so we have to subtract a week
                                // net result is to sometimes use ordinal and otherwise ordinal - 1
                                int weekCountTo = (dayCountTo < 0) ? ordinal : (ordinal - 1);

                                calendar.add(Calendar.WEEK_OF_MONTH, weekCountTo);

                                if (calendar.get(Calendar.MONTH) != month)
                                throw new RuntimeException("error3");


                                return calendar.getTime();



                                This avoids manually setting the number of days in the week and relies on Calendar to perform the math.






                                share|improve this answer

























                                  up vote
                                  0
                                  down vote













                                  Less iteration




                                  public static Date getDate(int n, int dayOfWeek, int month, int year)
                                  Calendar calendar = Calendar.getInstance();
                                  calendar.set(Calendar.YEAR, year);
                                  calendar.set(Calendar.MONTH, month);
                                  calendar.set(Calendar.DAY_OF_MONTH, 1);

                                  int matchCount = 0;

                                  while(true)
                                  if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
                                  matchCount++;

                                  if(matchCount == n)
                                  break;

                                  calendar.add(Calendar.DATE, 1);
                                  if(calendar.get(Calendar.MONTH) != month)
                                  break;



                                  if(matchCount != n)
                                  throw new RuntimeException("error");


                                  return calendar.getTime();




                                  Consider



                                  public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) ordinal <= 0) 
                                  throw new RuntimeException("error");


                                  return calendar.getTime();



                                  To me, a method named get returns a field from the object or class. This doesn't. Instead, it calculates a date for a particular input. I would find calculateDateFor sufficient, but you can write out something like calculateDateForOrdinalDayOfWeekInMonthYear if you prefer.



                                  The nth something is the ordinal number or ordinal for short. You may of course prefer ordinalNumber.



                                  In your original, you iterate by one each time. However, once on the right day of the week, you can iterate by seven.



                                  The name matchCount doesn't indicate what is being matched. So I changed it to weekCount instead.



                                  You check, after the fact, that the ordinal value was not too large. But you don't check that it isn't too small. I added that check to the exception gate. The original behavior was to print the first day of the month whenever the ordinal value is less than or equal to zero. This version changes that to be an exception instead.



                                  With math



                                  You use loops here, but that's not necessary. It's possible to get the same effect with just math.



                                  public static final int DAY_COUNT_PER_WEEK = 7;

                                  public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year)
                                  Calendar calendar = Calendar.getInstance();
                                  calendar.set(Calendar.YEAR, year);
                                  calendar.set(Calendar.MONTH, month);
                                  calendar.set(Calendar.DAY_OF_MONTH, 1);

                                  int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);

                                  // if the day of the week sought is before the day of the week
                                  // of the first day of the month, we need to add a week
                                  if (dayCountTo < 0)
                                  dayCountTo += DAY_COUNT_PER_WEEK;


                                  dayCountTo += DAY_COUNT_PER_WEEK * (ordinal - 1);

                                  calendar.add(Calendar.DATE, dayCountTo);

                                  if (calendar.get(Calendar.MONTH) != month)
                                  throw new RuntimeException("error");


                                  return calendar.getTime();



                                  Note that now if the ordinal number is too small or too large, it will produce an invalid month and trigger the exception. So we can remove the explicit check on ordinal.



                                  Alternately, consider



                                  public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) 
                                  Calendar calendar = Calendar.getInstance();
                                  calendar.set(Calendar.YEAR, year);
                                  calendar.set(Calendar.MONTH, month);
                                  calendar.set(Calendar.DAY_OF_MONTH, 1);

                                  int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);
                                  calendar.add(Calendar.DATE, dayCountTo);

                                  // if the day of the week sought is before the day of the week
                                  // of the first day of the month, we need to add a week
                                  // but ordinal is not zero-indexed, so we have to subtract a week
                                  // net result is to sometimes use ordinal and otherwise ordinal - 1
                                  int weekCountTo = (dayCountTo < 0) ? ordinal : (ordinal - 1);

                                  calendar.add(Calendar.WEEK_OF_MONTH, weekCountTo);

                                  if (calendar.get(Calendar.MONTH) != month)
                                  throw new RuntimeException("error3");


                                  return calendar.getTime();



                                  This avoids manually setting the number of days in the week and relies on Calendar to perform the math.






                                  share|improve this answer























                                    up vote
                                    0
                                    down vote










                                    up vote
                                    0
                                    down vote









                                    Less iteration




                                    public static Date getDate(int n, int dayOfWeek, int month, int year)
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int matchCount = 0;

                                    while(true)
                                    if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
                                    matchCount++;

                                    if(matchCount == n)
                                    break;

                                    calendar.add(Calendar.DATE, 1);
                                    if(calendar.get(Calendar.MONTH) != month)
                                    break;



                                    if(matchCount != n)
                                    throw new RuntimeException("error");


                                    return calendar.getTime();




                                    Consider



                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) ordinal <= 0) 
                                    throw new RuntimeException("error");


                                    return calendar.getTime();



                                    To me, a method named get returns a field from the object or class. This doesn't. Instead, it calculates a date for a particular input. I would find calculateDateFor sufficient, but you can write out something like calculateDateForOrdinalDayOfWeekInMonthYear if you prefer.



                                    The nth something is the ordinal number or ordinal for short. You may of course prefer ordinalNumber.



                                    In your original, you iterate by one each time. However, once on the right day of the week, you can iterate by seven.



                                    The name matchCount doesn't indicate what is being matched. So I changed it to weekCount instead.



                                    You check, after the fact, that the ordinal value was not too large. But you don't check that it isn't too small. I added that check to the exception gate. The original behavior was to print the first day of the month whenever the ordinal value is less than or equal to zero. This version changes that to be an exception instead.



                                    With math



                                    You use loops here, but that's not necessary. It's possible to get the same effect with just math.



                                    public static final int DAY_COUNT_PER_WEEK = 7;

                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year)
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);

                                    // if the day of the week sought is before the day of the week
                                    // of the first day of the month, we need to add a week
                                    if (dayCountTo < 0)
                                    dayCountTo += DAY_COUNT_PER_WEEK;


                                    dayCountTo += DAY_COUNT_PER_WEEK * (ordinal - 1);

                                    calendar.add(Calendar.DATE, dayCountTo);

                                    if (calendar.get(Calendar.MONTH) != month)
                                    throw new RuntimeException("error");


                                    return calendar.getTime();



                                    Note that now if the ordinal number is too small or too large, it will produce an invalid month and trigger the exception. So we can remove the explicit check on ordinal.



                                    Alternately, consider



                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) 
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);
                                    calendar.add(Calendar.DATE, dayCountTo);

                                    // if the day of the week sought is before the day of the week
                                    // of the first day of the month, we need to add a week
                                    // but ordinal is not zero-indexed, so we have to subtract a week
                                    // net result is to sometimes use ordinal and otherwise ordinal - 1
                                    int weekCountTo = (dayCountTo < 0) ? ordinal : (ordinal - 1);

                                    calendar.add(Calendar.WEEK_OF_MONTH, weekCountTo);

                                    if (calendar.get(Calendar.MONTH) != month)
                                    throw new RuntimeException("error3");


                                    return calendar.getTime();



                                    This avoids manually setting the number of days in the week and relies on Calendar to perform the math.






                                    share|improve this answer













                                    Less iteration




                                    public static Date getDate(int n, int dayOfWeek, int month, int year)
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int matchCount = 0;

                                    while(true)
                                    if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek)
                                    matchCount++;

                                    if(matchCount == n)
                                    break;

                                    calendar.add(Calendar.DATE, 1);
                                    if(calendar.get(Calendar.MONTH) != month)
                                    break;



                                    if(matchCount != n)
                                    throw new RuntimeException("error");


                                    return calendar.getTime();




                                    Consider



                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) ordinal <= 0) 
                                    throw new RuntimeException("error");


                                    return calendar.getTime();



                                    To me, a method named get returns a field from the object or class. This doesn't. Instead, it calculates a date for a particular input. I would find calculateDateFor sufficient, but you can write out something like calculateDateForOrdinalDayOfWeekInMonthYear if you prefer.



                                    The nth something is the ordinal number or ordinal for short. You may of course prefer ordinalNumber.



                                    In your original, you iterate by one each time. However, once on the right day of the week, you can iterate by seven.



                                    The name matchCount doesn't indicate what is being matched. So I changed it to weekCount instead.



                                    You check, after the fact, that the ordinal value was not too large. But you don't check that it isn't too small. I added that check to the exception gate. The original behavior was to print the first day of the month whenever the ordinal value is less than or equal to zero. This version changes that to be an exception instead.



                                    With math



                                    You use loops here, but that's not necessary. It's possible to get the same effect with just math.



                                    public static final int DAY_COUNT_PER_WEEK = 7;

                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year)
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);

                                    // if the day of the week sought is before the day of the week
                                    // of the first day of the month, we need to add a week
                                    if (dayCountTo < 0)
                                    dayCountTo += DAY_COUNT_PER_WEEK;


                                    dayCountTo += DAY_COUNT_PER_WEEK * (ordinal - 1);

                                    calendar.add(Calendar.DATE, dayCountTo);

                                    if (calendar.get(Calendar.MONTH) != month)
                                    throw new RuntimeException("error");


                                    return calendar.getTime();



                                    Note that now if the ordinal number is too small or too large, it will produce an invalid month and trigger the exception. So we can remove the explicit check on ordinal.



                                    Alternately, consider



                                    public static Date calculateDateFor(int ordinal, int dayOfWeek, int month, int year) 
                                    Calendar calendar = Calendar.getInstance();
                                    calendar.set(Calendar.YEAR, year);
                                    calendar.set(Calendar.MONTH, month);
                                    calendar.set(Calendar.DAY_OF_MONTH, 1);

                                    int dayCountTo = dayOfWeek - calendar.get(Calendar.DAY_OF_WEEK);
                                    calendar.add(Calendar.DATE, dayCountTo);

                                    // if the day of the week sought is before the day of the week
                                    // of the first day of the month, we need to add a week
                                    // but ordinal is not zero-indexed, so we have to subtract a week
                                    // net result is to sometimes use ordinal and otherwise ordinal - 1
                                    int weekCountTo = (dayCountTo < 0) ? ordinal : (ordinal - 1);

                                    calendar.add(Calendar.WEEK_OF_MONTH, weekCountTo);

                                    if (calendar.get(Calendar.MONTH) != month)
                                    throw new RuntimeException("error3");


                                    return calendar.getTime();



                                    This avoids manually setting the number of days in the week and relies on Calendar to perform the math.







                                    share|improve this answer













                                    share|improve this answer



                                    share|improve this answer











                                    answered Feb 26 at 5:53









                                    mdfst13

                                    16.8k42055




                                    16.8k42055






















                                         

                                        draft saved


                                        draft discarded


























                                         


                                        draft saved


                                        draft discarded














                                        StackExchange.ready(
                                        function ()
                                        StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188301%2fcalculate-date-for-ordinal-day-of-week-in-month%23new-answer', 'question_page');

                                        );

                                        Post as a guest













































































                                        Popular posts from this blog

                                        Greedy Best First Search implementation in Rust

                                        Function to Return a JSON Like Objects Using VBA Collections and Arrays

                                        C++11 CLH Lock Implementation