Finding decreasing sequences in an array of numbers











up vote
4
down vote

favorite













Ants



Requirement



Given a natural number $n$, representing the number of days the study is made, and then a number $x$ of $n$ natural numbers, representing the number of working ant flies after food, where the $x_i$ number of working ants is the day $i$. It is required to determine the number of maximum sequences, ordered strictly decreasing after the number of divisors, of minimum length $2$.



Input data



The input file furnici.in contains two lines. On the first line is the natural number $n$, with the meaning of the requirement and on the second line, the elements of the string $x$, with the meaning in the requirement.



Output data



The output file furnici.out will contain a natural number representing the number of maximum sequences, of minimum length $2$, ordered strictly downwards by the number of divisors.



Restrictions and clarifications



Time Limit - 0.5 seconds

Memory limit - 64 MB Total / 8 MB Stack
$2 leq n leq 100000 $
$10 leq x_i leq 1.000.000.000$



Example



furnici.in



10
719 169 4065 813 289 101 123 516 516 1017


furnici.out



2


Explanation



Row number of divisions is $2 3 8 4 3 2 4 12 12 6$.
It is noted that there are two maximum sequences, of at least two, strictly decreasing, $8 4 3 2$ and $12 6$.




My program is given an array of numbers that it has to find the divisors for and then replace the number in the array with its corresponding number of divisors. In the new created array, it will have to find and count all the descending subsequences that occur.



Here is my code:



#include <iostream>
#include <fstream>
#include <cmath>
using namespace std;
int n,i,v[100001],nr,j,s;
int main()
{
ifstream in("furnici.in");
ofstream out("furnici.out");
in>>n;
for(i=1; i<=n; i++)
{
in>>v[i];
nr=2;
for(j=2; j<=sqrt(v[i]); j++)
{
if(v[i]%j==0)
{
nr++;
if(j!=v[i]/j)
nr++;
}
}
v[i]=nr;
if(v[i]<v[i-1])
{
j=i;
while(v[j]<v[j-1] && j<n)
{
j++;
nr=2;
in>>v[j];
for(int k=2; k<=sqrt(v[j]); k++)
{
if(v[j]%k==0)
{
nr++;
if(k!=v[j]/k)
nr++;
}
}
v[j]=nr;
}
s++;
i=j;
}
}
out<<s;
return 0;
}


furnici.in



10
719 169 4065 813 289 101 123 516 516 1017


furnici.out



2


Explained:




  • 10 represents how many numbers are going to be read from the file;

  • number 719 has 2 divisors, so 719 gets replaced with 2;

  • number 169 has 3 divisors, so 169 gets replaced with 3;

  • number 4065 has 8 divisors, so 4065 gets replaced with 8;

  • and so on, you kind of get the point. The resulting array will have
    descending sequences: 2 3 8 4 3 2 4 12 12 6 and these need to be counted.


The issue with it is that it exceeds the time limit in the last test given to it, but otherwise the answer is correct.










share|improve this question




























    up vote
    4
    down vote

    favorite













    Ants



    Requirement



    Given a natural number $n$, representing the number of days the study is made, and then a number $x$ of $n$ natural numbers, representing the number of working ant flies after food, where the $x_i$ number of working ants is the day $i$. It is required to determine the number of maximum sequences, ordered strictly decreasing after the number of divisors, of minimum length $2$.



    Input data



    The input file furnici.in contains two lines. On the first line is the natural number $n$, with the meaning of the requirement and on the second line, the elements of the string $x$, with the meaning in the requirement.



    Output data



    The output file furnici.out will contain a natural number representing the number of maximum sequences, of minimum length $2$, ordered strictly downwards by the number of divisors.



    Restrictions and clarifications



    Time Limit - 0.5 seconds

    Memory limit - 64 MB Total / 8 MB Stack
    $2 leq n leq 100000 $
    $10 leq x_i leq 1.000.000.000$



    Example



    furnici.in



    10
    719 169 4065 813 289 101 123 516 516 1017


    furnici.out



    2


    Explanation



    Row number of divisions is $2 3 8 4 3 2 4 12 12 6$.
    It is noted that there are two maximum sequences, of at least two, strictly decreasing, $8 4 3 2$ and $12 6$.




    My program is given an array of numbers that it has to find the divisors for and then replace the number in the array with its corresponding number of divisors. In the new created array, it will have to find and count all the descending subsequences that occur.



    Here is my code:



    #include <iostream>
    #include <fstream>
    #include <cmath>
    using namespace std;
    int n,i,v[100001],nr,j,s;
    int main()
    {
    ifstream in("furnici.in");
    ofstream out("furnici.out");
    in>>n;
    for(i=1; i<=n; i++)
    {
    in>>v[i];
    nr=2;
    for(j=2; j<=sqrt(v[i]); j++)
    {
    if(v[i]%j==0)
    {
    nr++;
    if(j!=v[i]/j)
    nr++;
    }
    }
    v[i]=nr;
    if(v[i]<v[i-1])
    {
    j=i;
    while(v[j]<v[j-1] && j<n)
    {
    j++;
    nr=2;
    in>>v[j];
    for(int k=2; k<=sqrt(v[j]); k++)
    {
    if(v[j]%k==0)
    {
    nr++;
    if(k!=v[j]/k)
    nr++;
    }
    }
    v[j]=nr;
    }
    s++;
    i=j;
    }
    }
    out<<s;
    return 0;
    }


    furnici.in



    10
    719 169 4065 813 289 101 123 516 516 1017


    furnici.out



    2


    Explained:




    • 10 represents how many numbers are going to be read from the file;

    • number 719 has 2 divisors, so 719 gets replaced with 2;

    • number 169 has 3 divisors, so 169 gets replaced with 3;

    • number 4065 has 8 divisors, so 4065 gets replaced with 8;

    • and so on, you kind of get the point. The resulting array will have
      descending sequences: 2 3 8 4 3 2 4 12 12 6 and these need to be counted.


    The issue with it is that it exceeds the time limit in the last test given to it, but otherwise the answer is correct.










    share|improve this question


























      up vote
      4
      down vote

      favorite









      up vote
      4
      down vote

      favorite












      Ants



      Requirement



      Given a natural number $n$, representing the number of days the study is made, and then a number $x$ of $n$ natural numbers, representing the number of working ant flies after food, where the $x_i$ number of working ants is the day $i$. It is required to determine the number of maximum sequences, ordered strictly decreasing after the number of divisors, of minimum length $2$.



      Input data



      The input file furnici.in contains two lines. On the first line is the natural number $n$, with the meaning of the requirement and on the second line, the elements of the string $x$, with the meaning in the requirement.



      Output data



      The output file furnici.out will contain a natural number representing the number of maximum sequences, of minimum length $2$, ordered strictly downwards by the number of divisors.



      Restrictions and clarifications



      Time Limit - 0.5 seconds

      Memory limit - 64 MB Total / 8 MB Stack
      $2 leq n leq 100000 $
      $10 leq x_i leq 1.000.000.000$



      Example



      furnici.in



      10
      719 169 4065 813 289 101 123 516 516 1017


      furnici.out



      2


      Explanation



      Row number of divisions is $2 3 8 4 3 2 4 12 12 6$.
      It is noted that there are two maximum sequences, of at least two, strictly decreasing, $8 4 3 2$ and $12 6$.




      My program is given an array of numbers that it has to find the divisors for and then replace the number in the array with its corresponding number of divisors. In the new created array, it will have to find and count all the descending subsequences that occur.



      Here is my code:



      #include <iostream>
      #include <fstream>
      #include <cmath>
      using namespace std;
      int n,i,v[100001],nr,j,s;
      int main()
      {
      ifstream in("furnici.in");
      ofstream out("furnici.out");
      in>>n;
      for(i=1; i<=n; i++)
      {
      in>>v[i];
      nr=2;
      for(j=2; j<=sqrt(v[i]); j++)
      {
      if(v[i]%j==0)
      {
      nr++;
      if(j!=v[i]/j)
      nr++;
      }
      }
      v[i]=nr;
      if(v[i]<v[i-1])
      {
      j=i;
      while(v[j]<v[j-1] && j<n)
      {
      j++;
      nr=2;
      in>>v[j];
      for(int k=2; k<=sqrt(v[j]); k++)
      {
      if(v[j]%k==0)
      {
      nr++;
      if(k!=v[j]/k)
      nr++;
      }
      }
      v[j]=nr;
      }
      s++;
      i=j;
      }
      }
      out<<s;
      return 0;
      }


      furnici.in



      10
      719 169 4065 813 289 101 123 516 516 1017


      furnici.out



      2


      Explained:




      • 10 represents how many numbers are going to be read from the file;

      • number 719 has 2 divisors, so 719 gets replaced with 2;

      • number 169 has 3 divisors, so 169 gets replaced with 3;

      • number 4065 has 8 divisors, so 4065 gets replaced with 8;

      • and so on, you kind of get the point. The resulting array will have
        descending sequences: 2 3 8 4 3 2 4 12 12 6 and these need to be counted.


      The issue with it is that it exceeds the time limit in the last test given to it, but otherwise the answer is correct.










      share|improve this question
















      Ants



      Requirement



      Given a natural number $n$, representing the number of days the study is made, and then a number $x$ of $n$ natural numbers, representing the number of working ant flies after food, where the $x_i$ number of working ants is the day $i$. It is required to determine the number of maximum sequences, ordered strictly decreasing after the number of divisors, of minimum length $2$.



      Input data



      The input file furnici.in contains two lines. On the first line is the natural number $n$, with the meaning of the requirement and on the second line, the elements of the string $x$, with the meaning in the requirement.



      Output data



      The output file furnici.out will contain a natural number representing the number of maximum sequences, of minimum length $2$, ordered strictly downwards by the number of divisors.



      Restrictions and clarifications



      Time Limit - 0.5 seconds

      Memory limit - 64 MB Total / 8 MB Stack
      $2 leq n leq 100000 $
      $10 leq x_i leq 1.000.000.000$



      Example



      furnici.in



      10
      719 169 4065 813 289 101 123 516 516 1017


      furnici.out



      2


      Explanation



      Row number of divisions is $2 3 8 4 3 2 4 12 12 6$.
      It is noted that there are two maximum sequences, of at least two, strictly decreasing, $8 4 3 2$ and $12 6$.




      My program is given an array of numbers that it has to find the divisors for and then replace the number in the array with its corresponding number of divisors. In the new created array, it will have to find and count all the descending subsequences that occur.



      Here is my code:



      #include <iostream>
      #include <fstream>
      #include <cmath>
      using namespace std;
      int n,i,v[100001],nr,j,s;
      int main()
      {
      ifstream in("furnici.in");
      ofstream out("furnici.out");
      in>>n;
      for(i=1; i<=n; i++)
      {
      in>>v[i];
      nr=2;
      for(j=2; j<=sqrt(v[i]); j++)
      {
      if(v[i]%j==0)
      {
      nr++;
      if(j!=v[i]/j)
      nr++;
      }
      }
      v[i]=nr;
      if(v[i]<v[i-1])
      {
      j=i;
      while(v[j]<v[j-1] && j<n)
      {
      j++;
      nr=2;
      in>>v[j];
      for(int k=2; k<=sqrt(v[j]); k++)
      {
      if(v[j]%k==0)
      {
      nr++;
      if(k!=v[j]/k)
      nr++;
      }
      }
      v[j]=nr;
      }
      s++;
      i=j;
      }
      }
      out<<s;
      return 0;
      }


      furnici.in



      10
      719 169 4065 813 289 101 123 516 516 1017


      furnici.out



      2


      Explained:




      • 10 represents how many numbers are going to be read from the file;

      • number 719 has 2 divisors, so 719 gets replaced with 2;

      • number 169 has 3 divisors, so 169 gets replaced with 3;

      • number 4065 has 8 divisors, so 4065 gets replaced with 8;

      • and so on, you kind of get the point. The resulting array will have
        descending sequences: 2 3 8 4 3 2 4 12 12 6 and these need to be counted.


      The issue with it is that it exceeds the time limit in the last test given to it, but otherwise the answer is correct.







      c++ array time-limit-exceeded






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Nov 25 at 19:32









      Snowhawk

      5,15911028




      5,15911028










      asked Nov 25 at 11:14









      antoniu200

      234




      234






















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          4
          down vote



          accepted










          Some general remarks:




          • Don't use namespace std;, see for example Why is “using namespace std;” considered bad practice?.


          • Define variables at the narrowest possible scope. In particular, avoid
            global variables.



          • Use better variable names. It is unclear what each variable in



            int n,i,v[100001],nr,j,s;


            stands for.



          • return 0; at the end of the main program can be omitted.


          • The range of integer types is implementation defined, the C++ standard only
            guarantees that a (signed) int can hold values from -32767 to 32767,
            which is too small for your numbers. Many compilers define int as a 32-bit
            integer, but you can use long to be on the safe side, or use fixed-size
            types like int32_t.



          With respect to readability, I recommend to leave more (horizontal) space,
          e.g. around operators and parentheses.



          There are two places with identical code to count the divisors
          of a number. This should be done in a separate function.



          Your code uses a nested loop where the inner loop updates the index of the
          outer loop. That is difficult to understand and error-prone. And it is
          not necessary: Instead of starting a nested loop when the start of a
          decreasing subsequence is found, set a flag instead and continue with the
          main loop.



          You store all numbers from the input file in an array, which is not necessary:
          each loop iteration only needs the previous number to decide if the subsequence
          is (still) decreasing. It suffices to store the previously processed number
          in a variable.



          Summarizing the suggestions so far, the code could look like this:



          #include <fstream>
          #include <cmath>

          long numberOfDivisors(long n) {
          long count = 0;
          for (long j = 2; j <= sqrt(n); j++) {
          if (n % j == 0) {
          count++;
          if (j != n/j)
          count++;
          }
          }
          return count;
          }

          int main()
          {
          std::ifstream inFile("furnici.in");
          std::ofstream outFile("furnici.out");

          long decreasingSequences = 0;
          bool isDescending = false;
          long lastDivisorCount = 0;
          long numDays;
          inFile >> numDays;
          for (long i = 1; i <= numDays; i++) {
          long numAnts;
          inFile >> numAnts;
          long divisorCount = numberOfDivisors(numAnts);
          if (divisorCount >= lastDivisorCount) {
          // No longer decreasing.
          isDescending = false;
          } else if (!isDescending) {
          // A decreasing subsequence started right here.
          isDescending = true;
          decreasingSequences += 1;
          }
          lastDivisorCount = divisorCount;
          }
          outFile << decreasingSequences;
          }


          Now you can start to improve the performance, and the prime candidate is
          of course the numberOfDivisors() function.



          An efficient method (and I'm repeating arguments from Getting all divisors from an integer now) is to use the prime factorization: If
          $$
          n = p_1^{e_1} , p_2^{e_2} cdots p_k^{e_k}
          $$

          is the factorization of $ n $ into prime numbers $ p_i $
          with exponents $ e_i $, then
          $$
          sigma_0(n) = (e_1+1)(e_2+1) cdots (e_k+1)
          $$

          is the number of divisors of $ n $, see for example
          Wikipedia: Divisor function. Example:
          $$
          720 = 2^4 cdot 3^2 cdot 5^1 Longrightarrow
          sigma_0(720) = (4+1)(2+1)(1+1) = 30 , .
          $$



          Here is a possible implementation in C:



          long numberOfDivisors(long n){

          long numDivisors = 1;
          long factor = 2; // Candidate for prime factor of `n`

          // If `n` is not a prime number then it must have one factor
          // which is <= `sqrt(n)`, so we try these first:
          while (factor * factor <= n) {
          if (n % factor == 0) {
          // `factor` is a prime factor of `n`, determine the exponent:
          long exponent = 0;
          do {
          n /= factor;
          exponent++;
          } while (n % factor == 0);
          // `factor^exponent` is one term in the prime factorization of n,
          // this contributes as factor `exponent + 1`:
          numDivisors *= exponent + 1;
          }
          // Next possible prime factor:
          factor = factor == 2 ? 3 : factor + 2;
          }

          // Now `n` is either 1 or a prime number. In the latter case,
          // it contributes a factor 2:
          if (n > 1) {
          numDivisors *= 2;
          }

          return numDivisors;
          }


          As a further improvement you can pre-compute the prime numbers with a
          sieving method. Note that it sufficient to pre-compute the primes in the
          range $ 2 ldots sqrt{N} $ where $ N = 10^9 $ is the upper bound for the given input. That should help to stay within the given memory
          limit of 64 MB.






          share|improve this answer























          • This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
            – antoniu200
            Nov 25 at 20:09








          • 1




            @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
            – Martin R
            Nov 25 at 20:14










          • Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
            – antoniu200
            Nov 25 at 20:20












          • @antoniu200: It was Snowhawk who did the work on your question, not me :)
            – Martin R
            Nov 25 at 20:22










          • Whoops! You're right.
            – antoniu200
            Nov 25 at 20:24











          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: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          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%2f208368%2ffinding-decreasing-sequences-in-an-array-of-numbers%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown

























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          4
          down vote



          accepted










          Some general remarks:




          • Don't use namespace std;, see for example Why is “using namespace std;” considered bad practice?.


          • Define variables at the narrowest possible scope. In particular, avoid
            global variables.



          • Use better variable names. It is unclear what each variable in



            int n,i,v[100001],nr,j,s;


            stands for.



          • return 0; at the end of the main program can be omitted.


          • The range of integer types is implementation defined, the C++ standard only
            guarantees that a (signed) int can hold values from -32767 to 32767,
            which is too small for your numbers. Many compilers define int as a 32-bit
            integer, but you can use long to be on the safe side, or use fixed-size
            types like int32_t.



          With respect to readability, I recommend to leave more (horizontal) space,
          e.g. around operators and parentheses.



          There are two places with identical code to count the divisors
          of a number. This should be done in a separate function.



          Your code uses a nested loop where the inner loop updates the index of the
          outer loop. That is difficult to understand and error-prone. And it is
          not necessary: Instead of starting a nested loop when the start of a
          decreasing subsequence is found, set a flag instead and continue with the
          main loop.



          You store all numbers from the input file in an array, which is not necessary:
          each loop iteration only needs the previous number to decide if the subsequence
          is (still) decreasing. It suffices to store the previously processed number
          in a variable.



          Summarizing the suggestions so far, the code could look like this:



          #include <fstream>
          #include <cmath>

          long numberOfDivisors(long n) {
          long count = 0;
          for (long j = 2; j <= sqrt(n); j++) {
          if (n % j == 0) {
          count++;
          if (j != n/j)
          count++;
          }
          }
          return count;
          }

          int main()
          {
          std::ifstream inFile("furnici.in");
          std::ofstream outFile("furnici.out");

          long decreasingSequences = 0;
          bool isDescending = false;
          long lastDivisorCount = 0;
          long numDays;
          inFile >> numDays;
          for (long i = 1; i <= numDays; i++) {
          long numAnts;
          inFile >> numAnts;
          long divisorCount = numberOfDivisors(numAnts);
          if (divisorCount >= lastDivisorCount) {
          // No longer decreasing.
          isDescending = false;
          } else if (!isDescending) {
          // A decreasing subsequence started right here.
          isDescending = true;
          decreasingSequences += 1;
          }
          lastDivisorCount = divisorCount;
          }
          outFile << decreasingSequences;
          }


          Now you can start to improve the performance, and the prime candidate is
          of course the numberOfDivisors() function.



          An efficient method (and I'm repeating arguments from Getting all divisors from an integer now) is to use the prime factorization: If
          $$
          n = p_1^{e_1} , p_2^{e_2} cdots p_k^{e_k}
          $$

          is the factorization of $ n $ into prime numbers $ p_i $
          with exponents $ e_i $, then
          $$
          sigma_0(n) = (e_1+1)(e_2+1) cdots (e_k+1)
          $$

          is the number of divisors of $ n $, see for example
          Wikipedia: Divisor function. Example:
          $$
          720 = 2^4 cdot 3^2 cdot 5^1 Longrightarrow
          sigma_0(720) = (4+1)(2+1)(1+1) = 30 , .
          $$



          Here is a possible implementation in C:



          long numberOfDivisors(long n){

          long numDivisors = 1;
          long factor = 2; // Candidate for prime factor of `n`

          // If `n` is not a prime number then it must have one factor
          // which is <= `sqrt(n)`, so we try these first:
          while (factor * factor <= n) {
          if (n % factor == 0) {
          // `factor` is a prime factor of `n`, determine the exponent:
          long exponent = 0;
          do {
          n /= factor;
          exponent++;
          } while (n % factor == 0);
          // `factor^exponent` is one term in the prime factorization of n,
          // this contributes as factor `exponent + 1`:
          numDivisors *= exponent + 1;
          }
          // Next possible prime factor:
          factor = factor == 2 ? 3 : factor + 2;
          }

          // Now `n` is either 1 or a prime number. In the latter case,
          // it contributes a factor 2:
          if (n > 1) {
          numDivisors *= 2;
          }

          return numDivisors;
          }


          As a further improvement you can pre-compute the prime numbers with a
          sieving method. Note that it sufficient to pre-compute the primes in the
          range $ 2 ldots sqrt{N} $ where $ N = 10^9 $ is the upper bound for the given input. That should help to stay within the given memory
          limit of 64 MB.






          share|improve this answer























          • This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
            – antoniu200
            Nov 25 at 20:09








          • 1




            @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
            – Martin R
            Nov 25 at 20:14










          • Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
            – antoniu200
            Nov 25 at 20:20












          • @antoniu200: It was Snowhawk who did the work on your question, not me :)
            – Martin R
            Nov 25 at 20:22










          • Whoops! You're right.
            – antoniu200
            Nov 25 at 20:24















          up vote
          4
          down vote



          accepted










          Some general remarks:




          • Don't use namespace std;, see for example Why is “using namespace std;” considered bad practice?.


          • Define variables at the narrowest possible scope. In particular, avoid
            global variables.



          • Use better variable names. It is unclear what each variable in



            int n,i,v[100001],nr,j,s;


            stands for.



          • return 0; at the end of the main program can be omitted.


          • The range of integer types is implementation defined, the C++ standard only
            guarantees that a (signed) int can hold values from -32767 to 32767,
            which is too small for your numbers. Many compilers define int as a 32-bit
            integer, but you can use long to be on the safe side, or use fixed-size
            types like int32_t.



          With respect to readability, I recommend to leave more (horizontal) space,
          e.g. around operators and parentheses.



          There are two places with identical code to count the divisors
          of a number. This should be done in a separate function.



          Your code uses a nested loop where the inner loop updates the index of the
          outer loop. That is difficult to understand and error-prone. And it is
          not necessary: Instead of starting a nested loop when the start of a
          decreasing subsequence is found, set a flag instead and continue with the
          main loop.



          You store all numbers from the input file in an array, which is not necessary:
          each loop iteration only needs the previous number to decide if the subsequence
          is (still) decreasing. It suffices to store the previously processed number
          in a variable.



          Summarizing the suggestions so far, the code could look like this:



          #include <fstream>
          #include <cmath>

          long numberOfDivisors(long n) {
          long count = 0;
          for (long j = 2; j <= sqrt(n); j++) {
          if (n % j == 0) {
          count++;
          if (j != n/j)
          count++;
          }
          }
          return count;
          }

          int main()
          {
          std::ifstream inFile("furnici.in");
          std::ofstream outFile("furnici.out");

          long decreasingSequences = 0;
          bool isDescending = false;
          long lastDivisorCount = 0;
          long numDays;
          inFile >> numDays;
          for (long i = 1; i <= numDays; i++) {
          long numAnts;
          inFile >> numAnts;
          long divisorCount = numberOfDivisors(numAnts);
          if (divisorCount >= lastDivisorCount) {
          // No longer decreasing.
          isDescending = false;
          } else if (!isDescending) {
          // A decreasing subsequence started right here.
          isDescending = true;
          decreasingSequences += 1;
          }
          lastDivisorCount = divisorCount;
          }
          outFile << decreasingSequences;
          }


          Now you can start to improve the performance, and the prime candidate is
          of course the numberOfDivisors() function.



          An efficient method (and I'm repeating arguments from Getting all divisors from an integer now) is to use the prime factorization: If
          $$
          n = p_1^{e_1} , p_2^{e_2} cdots p_k^{e_k}
          $$

          is the factorization of $ n $ into prime numbers $ p_i $
          with exponents $ e_i $, then
          $$
          sigma_0(n) = (e_1+1)(e_2+1) cdots (e_k+1)
          $$

          is the number of divisors of $ n $, see for example
          Wikipedia: Divisor function. Example:
          $$
          720 = 2^4 cdot 3^2 cdot 5^1 Longrightarrow
          sigma_0(720) = (4+1)(2+1)(1+1) = 30 , .
          $$



          Here is a possible implementation in C:



          long numberOfDivisors(long n){

          long numDivisors = 1;
          long factor = 2; // Candidate for prime factor of `n`

          // If `n` is not a prime number then it must have one factor
          // which is <= `sqrt(n)`, so we try these first:
          while (factor * factor <= n) {
          if (n % factor == 0) {
          // `factor` is a prime factor of `n`, determine the exponent:
          long exponent = 0;
          do {
          n /= factor;
          exponent++;
          } while (n % factor == 0);
          // `factor^exponent` is one term in the prime factorization of n,
          // this contributes as factor `exponent + 1`:
          numDivisors *= exponent + 1;
          }
          // Next possible prime factor:
          factor = factor == 2 ? 3 : factor + 2;
          }

          // Now `n` is either 1 or a prime number. In the latter case,
          // it contributes a factor 2:
          if (n > 1) {
          numDivisors *= 2;
          }

          return numDivisors;
          }


          As a further improvement you can pre-compute the prime numbers with a
          sieving method. Note that it sufficient to pre-compute the primes in the
          range $ 2 ldots sqrt{N} $ where $ N = 10^9 $ is the upper bound for the given input. That should help to stay within the given memory
          limit of 64 MB.






          share|improve this answer























          • This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
            – antoniu200
            Nov 25 at 20:09








          • 1




            @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
            – Martin R
            Nov 25 at 20:14










          • Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
            – antoniu200
            Nov 25 at 20:20












          • @antoniu200: It was Snowhawk who did the work on your question, not me :)
            – Martin R
            Nov 25 at 20:22










          • Whoops! You're right.
            – antoniu200
            Nov 25 at 20:24













          up vote
          4
          down vote



          accepted







          up vote
          4
          down vote



          accepted






          Some general remarks:




          • Don't use namespace std;, see for example Why is “using namespace std;” considered bad practice?.


          • Define variables at the narrowest possible scope. In particular, avoid
            global variables.



          • Use better variable names. It is unclear what each variable in



            int n,i,v[100001],nr,j,s;


            stands for.



          • return 0; at the end of the main program can be omitted.


          • The range of integer types is implementation defined, the C++ standard only
            guarantees that a (signed) int can hold values from -32767 to 32767,
            which is too small for your numbers. Many compilers define int as a 32-bit
            integer, but you can use long to be on the safe side, or use fixed-size
            types like int32_t.



          With respect to readability, I recommend to leave more (horizontal) space,
          e.g. around operators and parentheses.



          There are two places with identical code to count the divisors
          of a number. This should be done in a separate function.



          Your code uses a nested loop where the inner loop updates the index of the
          outer loop. That is difficult to understand and error-prone. And it is
          not necessary: Instead of starting a nested loop when the start of a
          decreasing subsequence is found, set a flag instead and continue with the
          main loop.



          You store all numbers from the input file in an array, which is not necessary:
          each loop iteration only needs the previous number to decide if the subsequence
          is (still) decreasing. It suffices to store the previously processed number
          in a variable.



          Summarizing the suggestions so far, the code could look like this:



          #include <fstream>
          #include <cmath>

          long numberOfDivisors(long n) {
          long count = 0;
          for (long j = 2; j <= sqrt(n); j++) {
          if (n % j == 0) {
          count++;
          if (j != n/j)
          count++;
          }
          }
          return count;
          }

          int main()
          {
          std::ifstream inFile("furnici.in");
          std::ofstream outFile("furnici.out");

          long decreasingSequences = 0;
          bool isDescending = false;
          long lastDivisorCount = 0;
          long numDays;
          inFile >> numDays;
          for (long i = 1; i <= numDays; i++) {
          long numAnts;
          inFile >> numAnts;
          long divisorCount = numberOfDivisors(numAnts);
          if (divisorCount >= lastDivisorCount) {
          // No longer decreasing.
          isDescending = false;
          } else if (!isDescending) {
          // A decreasing subsequence started right here.
          isDescending = true;
          decreasingSequences += 1;
          }
          lastDivisorCount = divisorCount;
          }
          outFile << decreasingSequences;
          }


          Now you can start to improve the performance, and the prime candidate is
          of course the numberOfDivisors() function.



          An efficient method (and I'm repeating arguments from Getting all divisors from an integer now) is to use the prime factorization: If
          $$
          n = p_1^{e_1} , p_2^{e_2} cdots p_k^{e_k}
          $$

          is the factorization of $ n $ into prime numbers $ p_i $
          with exponents $ e_i $, then
          $$
          sigma_0(n) = (e_1+1)(e_2+1) cdots (e_k+1)
          $$

          is the number of divisors of $ n $, see for example
          Wikipedia: Divisor function. Example:
          $$
          720 = 2^4 cdot 3^2 cdot 5^1 Longrightarrow
          sigma_0(720) = (4+1)(2+1)(1+1) = 30 , .
          $$



          Here is a possible implementation in C:



          long numberOfDivisors(long n){

          long numDivisors = 1;
          long factor = 2; // Candidate for prime factor of `n`

          // If `n` is not a prime number then it must have one factor
          // which is <= `sqrt(n)`, so we try these first:
          while (factor * factor <= n) {
          if (n % factor == 0) {
          // `factor` is a prime factor of `n`, determine the exponent:
          long exponent = 0;
          do {
          n /= factor;
          exponent++;
          } while (n % factor == 0);
          // `factor^exponent` is one term in the prime factorization of n,
          // this contributes as factor `exponent + 1`:
          numDivisors *= exponent + 1;
          }
          // Next possible prime factor:
          factor = factor == 2 ? 3 : factor + 2;
          }

          // Now `n` is either 1 or a prime number. In the latter case,
          // it contributes a factor 2:
          if (n > 1) {
          numDivisors *= 2;
          }

          return numDivisors;
          }


          As a further improvement you can pre-compute the prime numbers with a
          sieving method. Note that it sufficient to pre-compute the primes in the
          range $ 2 ldots sqrt{N} $ where $ N = 10^9 $ is the upper bound for the given input. That should help to stay within the given memory
          limit of 64 MB.






          share|improve this answer














          Some general remarks:




          • Don't use namespace std;, see for example Why is “using namespace std;” considered bad practice?.


          • Define variables at the narrowest possible scope. In particular, avoid
            global variables.



          • Use better variable names. It is unclear what each variable in



            int n,i,v[100001],nr,j,s;


            stands for.



          • return 0; at the end of the main program can be omitted.


          • The range of integer types is implementation defined, the C++ standard only
            guarantees that a (signed) int can hold values from -32767 to 32767,
            which is too small for your numbers. Many compilers define int as a 32-bit
            integer, but you can use long to be on the safe side, or use fixed-size
            types like int32_t.



          With respect to readability, I recommend to leave more (horizontal) space,
          e.g. around operators and parentheses.



          There are two places with identical code to count the divisors
          of a number. This should be done in a separate function.



          Your code uses a nested loop where the inner loop updates the index of the
          outer loop. That is difficult to understand and error-prone. And it is
          not necessary: Instead of starting a nested loop when the start of a
          decreasing subsequence is found, set a flag instead and continue with the
          main loop.



          You store all numbers from the input file in an array, which is not necessary:
          each loop iteration only needs the previous number to decide if the subsequence
          is (still) decreasing. It suffices to store the previously processed number
          in a variable.



          Summarizing the suggestions so far, the code could look like this:



          #include <fstream>
          #include <cmath>

          long numberOfDivisors(long n) {
          long count = 0;
          for (long j = 2; j <= sqrt(n); j++) {
          if (n % j == 0) {
          count++;
          if (j != n/j)
          count++;
          }
          }
          return count;
          }

          int main()
          {
          std::ifstream inFile("furnici.in");
          std::ofstream outFile("furnici.out");

          long decreasingSequences = 0;
          bool isDescending = false;
          long lastDivisorCount = 0;
          long numDays;
          inFile >> numDays;
          for (long i = 1; i <= numDays; i++) {
          long numAnts;
          inFile >> numAnts;
          long divisorCount = numberOfDivisors(numAnts);
          if (divisorCount >= lastDivisorCount) {
          // No longer decreasing.
          isDescending = false;
          } else if (!isDescending) {
          // A decreasing subsequence started right here.
          isDescending = true;
          decreasingSequences += 1;
          }
          lastDivisorCount = divisorCount;
          }
          outFile << decreasingSequences;
          }


          Now you can start to improve the performance, and the prime candidate is
          of course the numberOfDivisors() function.



          An efficient method (and I'm repeating arguments from Getting all divisors from an integer now) is to use the prime factorization: If
          $$
          n = p_1^{e_1} , p_2^{e_2} cdots p_k^{e_k}
          $$

          is the factorization of $ n $ into prime numbers $ p_i $
          with exponents $ e_i $, then
          $$
          sigma_0(n) = (e_1+1)(e_2+1) cdots (e_k+1)
          $$

          is the number of divisors of $ n $, see for example
          Wikipedia: Divisor function. Example:
          $$
          720 = 2^4 cdot 3^2 cdot 5^1 Longrightarrow
          sigma_0(720) = (4+1)(2+1)(1+1) = 30 , .
          $$



          Here is a possible implementation in C:



          long numberOfDivisors(long n){

          long numDivisors = 1;
          long factor = 2; // Candidate for prime factor of `n`

          // If `n` is not a prime number then it must have one factor
          // which is <= `sqrt(n)`, so we try these first:
          while (factor * factor <= n) {
          if (n % factor == 0) {
          // `factor` is a prime factor of `n`, determine the exponent:
          long exponent = 0;
          do {
          n /= factor;
          exponent++;
          } while (n % factor == 0);
          // `factor^exponent` is one term in the prime factorization of n,
          // this contributes as factor `exponent + 1`:
          numDivisors *= exponent + 1;
          }
          // Next possible prime factor:
          factor = factor == 2 ? 3 : factor + 2;
          }

          // Now `n` is either 1 or a prime number. In the latter case,
          // it contributes a factor 2:
          if (n > 1) {
          numDivisors *= 2;
          }

          return numDivisors;
          }


          As a further improvement you can pre-compute the prime numbers with a
          sieving method. Note that it sufficient to pre-compute the primes in the
          range $ 2 ldots sqrt{N} $ where $ N = 10^9 $ is the upper bound for the given input. That should help to stay within the given memory
          limit of 64 MB.







          share|improve this answer














          share|improve this answer



          share|improve this answer








          edited Nov 25 at 20:15

























          answered Nov 25 at 19:32









          Martin R

          15.4k12264




          15.4k12264












          • This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
            – antoniu200
            Nov 25 at 20:09








          • 1




            @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
            – Martin R
            Nov 25 at 20:14










          • Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
            – antoniu200
            Nov 25 at 20:20












          • @antoniu200: It was Snowhawk who did the work on your question, not me :)
            – Martin R
            Nov 25 at 20:22










          • Whoops! You're right.
            – antoniu200
            Nov 25 at 20:24


















          • This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
            – antoniu200
            Nov 25 at 20:09








          • 1




            @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
            – Martin R
            Nov 25 at 20:14










          • Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
            – antoniu200
            Nov 25 at 20:20












          • @antoniu200: It was Snowhawk who did the work on your question, not me :)
            – Martin R
            Nov 25 at 20:22










          • Whoops! You're right.
            – antoniu200
            Nov 25 at 20:24
















          This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
          – antoniu200
          Nov 25 at 20:09






          This does solve the time issue, but I have a question, as I haven't seen this before: what does this "factor = factor == 2 ? 3 : factor + 2;" line do? Most notably, the question mark.
          – antoniu200
          Nov 25 at 20:09






          1




          1




          @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
          – Martin R
          Nov 25 at 20:14




          @antoniu200: That is the “conditional operator” (some people call it “ternary operator”). condition ? expr1 : expr2 evaluates to expr1 if the condition is true, and to expr2 otherwise. Here it is a shorthand for if (factor == 2) { factor = 3 } else { factor = factor + 2 }
          – Martin R
          Nov 25 at 20:14












          Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
          – antoniu200
          Nov 25 at 20:20






          Thank you very much! I also noticed you edited the SO completely. Thanks for that too, now I know exactly what to include from now on!
          – antoniu200
          Nov 25 at 20:20














          @antoniu200: It was Snowhawk who did the work on your question, not me :)
          – Martin R
          Nov 25 at 20:22




          @antoniu200: It was Snowhawk who did the work on your question, not me :)
          – Martin R
          Nov 25 at 20:22












          Whoops! You're right.
          – antoniu200
          Nov 25 at 20:24




          Whoops! You're right.
          – antoniu200
          Nov 25 at 20:24


















          draft saved

          draft discarded




















































          Thanks for contributing an answer to Code Review Stack Exchange!


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          Use MathJax to format equations. MathJax reference.


          To learn more, see our tips on writing great answers.





          Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


          Please pay close attention to the following guidance:


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          To learn more, see our tips on writing great answers.




          draft saved


          draft discarded














          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208368%2ffinding-decreasing-sequences-in-an-array-of-numbers%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown





















































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown

































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown







          Popular posts from this blog

          Сан-Квентин

          Алькесар

          Josef Freinademetz