Project Euler Problem #5 Solution in C++

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











up vote
1
down vote

favorite













Project Euler #5: Smallest multiple



2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.



What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?"




And I'm wondering if my code to solve this problem is good and if there are ways to make it better.



#include "stdafx.h"
#include <iostream>
int main()
int smallestMultiple = 40;
int i = 10;

while (i<20)
if (smallestMultiple%i == 0)
i++;
continue;

else
i = 10;
smallestMultiple += 20;


std::cout << smallestMultiple;










share|improve this question



















  • 3




    There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
    – Martin R
    Aug 23 at 13:20














up vote
1
down vote

favorite













Project Euler #5: Smallest multiple



2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.



What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?"




And I'm wondering if my code to solve this problem is good and if there are ways to make it better.



#include "stdafx.h"
#include <iostream>
int main()
int smallestMultiple = 40;
int i = 10;

while (i<20)
if (smallestMultiple%i == 0)
i++;
continue;

else
i = 10;
smallestMultiple += 20;


std::cout << smallestMultiple;










share|improve this question



















  • 3




    There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
    – Martin R
    Aug 23 at 13:20












up vote
1
down vote

favorite









up vote
1
down vote

favorite












Project Euler #5: Smallest multiple



2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.



What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?"




And I'm wondering if my code to solve this problem is good and if there are ways to make it better.



#include "stdafx.h"
#include <iostream>
int main()
int smallestMultiple = 40;
int i = 10;

while (i<20)
if (smallestMultiple%i == 0)
i++;
continue;

else
i = 10;
smallestMultiple += 20;


std::cout << smallestMultiple;










share|improve this question
















Project Euler #5: Smallest multiple



2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.



What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?"




And I'm wondering if my code to solve this problem is good and if there are ways to make it better.



#include "stdafx.h"
#include <iostream>
int main()
int smallestMultiple = 40;
int i = 10;

while (i<20)
if (smallestMultiple%i == 0)
i++;
continue;

else
i = 10;
smallestMultiple += 20;


std::cout << smallestMultiple;







c++ programming-challenge






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Aug 23 at 14:14









Deduplicator

10.4k1849




10.4k1849










asked Aug 23 at 13:09









Phenol

112




112







  • 3




    There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
    – Martin R
    Aug 23 at 13:20












  • 3




    There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
    – Martin R
    Aug 23 at 13:20







3




3




There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
– Martin R
Aug 23 at 13:20




There are far more efficient ways to get the result. I would suggest to have a look at existing Q&A's about the same problem, such as codereview.stackexchange.com/q/80500/35991.
– Martin R
Aug 23 at 13:20










3 Answers
3






active

oldest

votes

















up vote
10
down vote



accepted










First reviewing the code itself:



  1. First, fix the indentation, and insert an empty line before your main(). Proper formatting is a most basic step allowing easier comprehension.


  2. "stdafx.h" is part of the MSVC-support for pre-compiled headers. Your project is so small it cannot really take advantage of that anyway, so it's just a wart.


  3. Use the right loop for the job. A for-loop seems to fit better, as you have init (with declaration), condition, next, and body.


  4. Don't use contine where it has no effect, nor clarifies intent.


  5. In general, int is too small for your result. Luckily(?), on Windows it's 32 bit, which should suffice.


  6. Put the calculation into its own function for re-use.


  7. Don't forget to output a newline at the end, it's expected and omitting it might have interesting effects.


And then the algorithm:



Testing every 20th number whether it's the target is supremely inefficient. Even using steps of 2520 instead isn't that much better.

Consider constructing the result instead:




  1. r = 1

  2. For i = 2 to the limit

    • If i does not divide r

      • Multiply r by the highest integer-power of i not exceeding the limit.



Keeping in mind that Project Euler is not really for brute-force or straight-up coding, but figuring out an intelligent approach for manually calculating things:



  1. 2520 is the least common multiple of the numbers 1 through 10.

  2. Go through 11 to 20, and figure out whether you need an additional factor.

    Observation: 4 times the number itself (it's prime), once the fourth root.

  3. That's easily put into a calculator, for the result 232792560.

Putting it all together:



#include <iostream>

static unsigned long lcm_until(unsigned n) noexcept
auto r = 1UL;
for (auto i = 2UL; i <= n; ++i)
if (r % i) // i is prime
auto x = i;
while (x * i <= n)
x *= i;
r *= x;


return r;


int main()
std::cout << lcm_until(20) << 'n';






share|improve this answer


















  • 1




    Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
    – Toby Speight
    Aug 23 at 15:58

















up vote
10
down vote













Code Review



Despite its name, "stdafx.h" isn't a standard header. It doesn't seem to be needed, as the code compiles and runs without it.



Your indentation is off - perhaps that's a result of how you inserted the code into the question?



The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.



The code assumes that a result will be found before int overflows. Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?



Your loop structure would be clearer if you separate out the test from incrementing smallest multiple. That could look something like this (still using int as our type):



#include <iostream>
#include <climits>

static bool is_multiple_of_all(int n, int max)

for (int i = 10; i <= max; ++i)
if (n % i != 0)
return false;


return true;



int main()

const int target = 20;
for (int candidate = 2 * target;
candidate <= INT_MAX - target;
candidate += target)

if (is_multiple_of_all(candidate, target))
std::cout << candidate;
return 0;


// not found
return 1;



Algorithm



Project Euler problems tend to exercise your mathematical reasoning. You're expected to find better methods than brute force search to find the solution. (Hint: think about prime factors).






share|improve this answer





























    up vote
    1
    down vote













    Another way to approach this, is to use the std::lcm(m, n) (c++17) function. Start with 2520 and 11:



    #include <numeric>
    using std::lcm;
    typedef unsigned long long ull;
    ull solve()

    static const int limit = 20;
    static int current = 11;
    static ull answer = 2520;
    if (current > limit)

    return answer;

    answer = lcm(answer, current++);
    return solve();






    share|improve this answer






















    • That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
      – Toby Speight
      Aug 24 at 6:42







    • 3




      static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
      – Deduplicator
      Aug 24 at 11: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: 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%2f202307%2fproject-euler-problem-5-solution-in-c%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
    10
    down vote



    accepted










    First reviewing the code itself:



    1. First, fix the indentation, and insert an empty line before your main(). Proper formatting is a most basic step allowing easier comprehension.


    2. "stdafx.h" is part of the MSVC-support for pre-compiled headers. Your project is so small it cannot really take advantage of that anyway, so it's just a wart.


    3. Use the right loop for the job. A for-loop seems to fit better, as you have init (with declaration), condition, next, and body.


    4. Don't use contine where it has no effect, nor clarifies intent.


    5. In general, int is too small for your result. Luckily(?), on Windows it's 32 bit, which should suffice.


    6. Put the calculation into its own function for re-use.


    7. Don't forget to output a newline at the end, it's expected and omitting it might have interesting effects.


    And then the algorithm:



    Testing every 20th number whether it's the target is supremely inefficient. Even using steps of 2520 instead isn't that much better.

    Consider constructing the result instead:




    1. r = 1

    2. For i = 2 to the limit

      • If i does not divide r

        • Multiply r by the highest integer-power of i not exceeding the limit.



    Keeping in mind that Project Euler is not really for brute-force or straight-up coding, but figuring out an intelligent approach for manually calculating things:



    1. 2520 is the least common multiple of the numbers 1 through 10.

    2. Go through 11 to 20, and figure out whether you need an additional factor.

      Observation: 4 times the number itself (it's prime), once the fourth root.

    3. That's easily put into a calculator, for the result 232792560.

    Putting it all together:



    #include <iostream>

    static unsigned long lcm_until(unsigned n) noexcept
    auto r = 1UL;
    for (auto i = 2UL; i <= n; ++i)
    if (r % i) // i is prime
    auto x = i;
    while (x * i <= n)
    x *= i;
    r *= x;


    return r;


    int main()
    std::cout << lcm_until(20) << 'n';






    share|improve this answer


















    • 1




      Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
      – Toby Speight
      Aug 23 at 15:58














    up vote
    10
    down vote



    accepted










    First reviewing the code itself:



    1. First, fix the indentation, and insert an empty line before your main(). Proper formatting is a most basic step allowing easier comprehension.


    2. "stdafx.h" is part of the MSVC-support for pre-compiled headers. Your project is so small it cannot really take advantage of that anyway, so it's just a wart.


    3. Use the right loop for the job. A for-loop seems to fit better, as you have init (with declaration), condition, next, and body.


    4. Don't use contine where it has no effect, nor clarifies intent.


    5. In general, int is too small for your result. Luckily(?), on Windows it's 32 bit, which should suffice.


    6. Put the calculation into its own function for re-use.


    7. Don't forget to output a newline at the end, it's expected and omitting it might have interesting effects.


    And then the algorithm:



    Testing every 20th number whether it's the target is supremely inefficient. Even using steps of 2520 instead isn't that much better.

    Consider constructing the result instead:




    1. r = 1

    2. For i = 2 to the limit

      • If i does not divide r

        • Multiply r by the highest integer-power of i not exceeding the limit.



    Keeping in mind that Project Euler is not really for brute-force or straight-up coding, but figuring out an intelligent approach for manually calculating things:



    1. 2520 is the least common multiple of the numbers 1 through 10.

    2. Go through 11 to 20, and figure out whether you need an additional factor.

      Observation: 4 times the number itself (it's prime), once the fourth root.

    3. That's easily put into a calculator, for the result 232792560.

    Putting it all together:



    #include <iostream>

    static unsigned long lcm_until(unsigned n) noexcept
    auto r = 1UL;
    for (auto i = 2UL; i <= n; ++i)
    if (r % i) // i is prime
    auto x = i;
    while (x * i <= n)
    x *= i;
    r *= x;


    return r;


    int main()
    std::cout << lcm_until(20) << 'n';






    share|improve this answer


















    • 1




      Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
      – Toby Speight
      Aug 23 at 15:58












    up vote
    10
    down vote



    accepted







    up vote
    10
    down vote



    accepted






    First reviewing the code itself:



    1. First, fix the indentation, and insert an empty line before your main(). Proper formatting is a most basic step allowing easier comprehension.


    2. "stdafx.h" is part of the MSVC-support for pre-compiled headers. Your project is so small it cannot really take advantage of that anyway, so it's just a wart.


    3. Use the right loop for the job. A for-loop seems to fit better, as you have init (with declaration), condition, next, and body.


    4. Don't use contine where it has no effect, nor clarifies intent.


    5. In general, int is too small for your result. Luckily(?), on Windows it's 32 bit, which should suffice.


    6. Put the calculation into its own function for re-use.


    7. Don't forget to output a newline at the end, it's expected and omitting it might have interesting effects.


    And then the algorithm:



    Testing every 20th number whether it's the target is supremely inefficient. Even using steps of 2520 instead isn't that much better.

    Consider constructing the result instead:




    1. r = 1

    2. For i = 2 to the limit

      • If i does not divide r

        • Multiply r by the highest integer-power of i not exceeding the limit.



    Keeping in mind that Project Euler is not really for brute-force or straight-up coding, but figuring out an intelligent approach for manually calculating things:



    1. 2520 is the least common multiple of the numbers 1 through 10.

    2. Go through 11 to 20, and figure out whether you need an additional factor.

      Observation: 4 times the number itself (it's prime), once the fourth root.

    3. That's easily put into a calculator, for the result 232792560.

    Putting it all together:



    #include <iostream>

    static unsigned long lcm_until(unsigned n) noexcept
    auto r = 1UL;
    for (auto i = 2UL; i <= n; ++i)
    if (r % i) // i is prime
    auto x = i;
    while (x * i <= n)
    x *= i;
    r *= x;


    return r;


    int main()
    std::cout << lcm_until(20) << 'n';






    share|improve this answer














    First reviewing the code itself:



    1. First, fix the indentation, and insert an empty line before your main(). Proper formatting is a most basic step allowing easier comprehension.


    2. "stdafx.h" is part of the MSVC-support for pre-compiled headers. Your project is so small it cannot really take advantage of that anyway, so it's just a wart.


    3. Use the right loop for the job. A for-loop seems to fit better, as you have init (with declaration), condition, next, and body.


    4. Don't use contine where it has no effect, nor clarifies intent.


    5. In general, int is too small for your result. Luckily(?), on Windows it's 32 bit, which should suffice.


    6. Put the calculation into its own function for re-use.


    7. Don't forget to output a newline at the end, it's expected and omitting it might have interesting effects.


    And then the algorithm:



    Testing every 20th number whether it's the target is supremely inefficient. Even using steps of 2520 instead isn't that much better.

    Consider constructing the result instead:




    1. r = 1

    2. For i = 2 to the limit

      • If i does not divide r

        • Multiply r by the highest integer-power of i not exceeding the limit.



    Keeping in mind that Project Euler is not really for brute-force or straight-up coding, but figuring out an intelligent approach for manually calculating things:



    1. 2520 is the least common multiple of the numbers 1 through 10.

    2. Go through 11 to 20, and figure out whether you need an additional factor.

      Observation: 4 times the number itself (it's prime), once the fourth root.

    3. That's easily put into a calculator, for the result 232792560.

    Putting it all together:



    #include <iostream>

    static unsigned long lcm_until(unsigned n) noexcept
    auto r = 1UL;
    for (auto i = 2UL; i <= n; ++i)
    if (r % i) // i is prime
    auto x = i;
    while (x * i <= n)
    x *= i;
    r *= x;


    return r;


    int main()
    std::cout << lcm_until(20) << 'n';







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited Aug 23 at 20:25

























    answered Aug 23 at 14:07









    Deduplicator

    10.4k1849




    10.4k1849







    • 1




      Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
      – Toby Speight
      Aug 23 at 15:58












    • 1




      Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
      – Toby Speight
      Aug 23 at 15:58







    1




    1




    Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
    – Toby Speight
    Aug 23 at 15:58




    Much better code - it does break down without warning after n=115 on this system with 64-bit unsigned long (or after n=159 with 128-bit unsigned long). Of course, the original code would take years to get to those values...
    – Toby Speight
    Aug 23 at 15:58












    up vote
    10
    down vote













    Code Review



    Despite its name, "stdafx.h" isn't a standard header. It doesn't seem to be needed, as the code compiles and runs without it.



    Your indentation is off - perhaps that's a result of how you inserted the code into the question?



    The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.



    The code assumes that a result will be found before int overflows. Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?



    Your loop structure would be clearer if you separate out the test from incrementing smallest multiple. That could look something like this (still using int as our type):



    #include <iostream>
    #include <climits>

    static bool is_multiple_of_all(int n, int max)

    for (int i = 10; i <= max; ++i)
    if (n % i != 0)
    return false;


    return true;



    int main()

    const int target = 20;
    for (int candidate = 2 * target;
    candidate <= INT_MAX - target;
    candidate += target)

    if (is_multiple_of_all(candidate, target))
    std::cout << candidate;
    return 0;


    // not found
    return 1;



    Algorithm



    Project Euler problems tend to exercise your mathematical reasoning. You're expected to find better methods than brute force search to find the solution. (Hint: think about prime factors).






    share|improve this answer


























      up vote
      10
      down vote













      Code Review



      Despite its name, "stdafx.h" isn't a standard header. It doesn't seem to be needed, as the code compiles and runs without it.



      Your indentation is off - perhaps that's a result of how you inserted the code into the question?



      The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.



      The code assumes that a result will be found before int overflows. Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?



      Your loop structure would be clearer if you separate out the test from incrementing smallest multiple. That could look something like this (still using int as our type):



      #include <iostream>
      #include <climits>

      static bool is_multiple_of_all(int n, int max)

      for (int i = 10; i <= max; ++i)
      if (n % i != 0)
      return false;


      return true;



      int main()

      const int target = 20;
      for (int candidate = 2 * target;
      candidate <= INT_MAX - target;
      candidate += target)

      if (is_multiple_of_all(candidate, target))
      std::cout << candidate;
      return 0;


      // not found
      return 1;



      Algorithm



      Project Euler problems tend to exercise your mathematical reasoning. You're expected to find better methods than brute force search to find the solution. (Hint: think about prime factors).






      share|improve this answer
























        up vote
        10
        down vote










        up vote
        10
        down vote









        Code Review



        Despite its name, "stdafx.h" isn't a standard header. It doesn't seem to be needed, as the code compiles and runs without it.



        Your indentation is off - perhaps that's a result of how you inserted the code into the question?



        The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.



        The code assumes that a result will be found before int overflows. Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?



        Your loop structure would be clearer if you separate out the test from incrementing smallest multiple. That could look something like this (still using int as our type):



        #include <iostream>
        #include <climits>

        static bool is_multiple_of_all(int n, int max)

        for (int i = 10; i <= max; ++i)
        if (n % i != 0)
        return false;


        return true;



        int main()

        const int target = 20;
        for (int candidate = 2 * target;
        candidate <= INT_MAX - target;
        candidate += target)

        if (is_multiple_of_all(candidate, target))
        std::cout << candidate;
        return 0;


        // not found
        return 1;



        Algorithm



        Project Euler problems tend to exercise your mathematical reasoning. You're expected to find better methods than brute force search to find the solution. (Hint: think about prime factors).






        share|improve this answer














        Code Review



        Despite its name, "stdafx.h" isn't a standard header. It doesn't seem to be needed, as the code compiles and runs without it.



        Your indentation is off - perhaps that's a result of how you inserted the code into the question?



        The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.



        The code assumes that a result will be found before int overflows. Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?



        Your loop structure would be clearer if you separate out the test from incrementing smallest multiple. That could look something like this (still using int as our type):



        #include <iostream>
        #include <climits>

        static bool is_multiple_of_all(int n, int max)

        for (int i = 10; i <= max; ++i)
        if (n % i != 0)
        return false;


        return true;



        int main()

        const int target = 20;
        for (int candidate = 2 * target;
        candidate <= INT_MAX - target;
        candidate += target)

        if (is_multiple_of_all(candidate, target))
        std::cout << candidate;
        return 0;


        // not found
        return 1;



        Algorithm



        Project Euler problems tend to exercise your mathematical reasoning. You're expected to find better methods than brute force search to find the solution. (Hint: think about prime factors).







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited Aug 23 at 13:54

























        answered Aug 23 at 13:36









        Toby Speight

        19.3k43698




        19.3k43698




















            up vote
            1
            down vote













            Another way to approach this, is to use the std::lcm(m, n) (c++17) function. Start with 2520 and 11:



            #include <numeric>
            using std::lcm;
            typedef unsigned long long ull;
            ull solve()

            static const int limit = 20;
            static int current = 11;
            static ull answer = 2520;
            if (current > limit)

            return answer;

            answer = lcm(answer, current++);
            return solve();






            share|improve this answer






















            • That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
              – Toby Speight
              Aug 24 at 6:42







            • 3




              static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
              – Deduplicator
              Aug 24 at 11:24














            up vote
            1
            down vote













            Another way to approach this, is to use the std::lcm(m, n) (c++17) function. Start with 2520 and 11:



            #include <numeric>
            using std::lcm;
            typedef unsigned long long ull;
            ull solve()

            static const int limit = 20;
            static int current = 11;
            static ull answer = 2520;
            if (current > limit)

            return answer;

            answer = lcm(answer, current++);
            return solve();






            share|improve this answer






















            • That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
              – Toby Speight
              Aug 24 at 6:42







            • 3




              static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
              – Deduplicator
              Aug 24 at 11:24












            up vote
            1
            down vote










            up vote
            1
            down vote









            Another way to approach this, is to use the std::lcm(m, n) (c++17) function. Start with 2520 and 11:



            #include <numeric>
            using std::lcm;
            typedef unsigned long long ull;
            ull solve()

            static const int limit = 20;
            static int current = 11;
            static ull answer = 2520;
            if (current > limit)

            return answer;

            answer = lcm(answer, current++);
            return solve();






            share|improve this answer














            Another way to approach this, is to use the std::lcm(m, n) (c++17) function. Start with 2520 and 11:



            #include <numeric>
            using std::lcm;
            typedef unsigned long long ull;
            ull solve()

            static const int limit = 20;
            static int current = 11;
            static ull answer = 2520;
            if (current > limit)

            return answer;

            answer = lcm(answer, current++);
            return solve();







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited Aug 24 at 11:18









            Deduplicator

            10.4k1849




            10.4k1849










            answered Aug 24 at 5:14









            tinstaafl

            5,692625




            5,692625











            • That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
              – Toby Speight
              Aug 24 at 6:42







            • 3




              static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
              – Deduplicator
              Aug 24 at 11:24
















            • That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
              – Toby Speight
              Aug 24 at 6:42







            • 3




              static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
              – Deduplicator
              Aug 24 at 11:24















            That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
            – Toby Speight
            Aug 24 at 6:42





            That appears to be a single-use function. I wouldn't advise that, because it makes it hard to generalise to one that accepts an argument. Besides, it's much clearer in iterative form: for (auto current = 2u; current <= limit; ++current) answer = std::lcm(answer, current++);. Do note that std::lcm() gives wrong results from only limit==86, so we have less range when we use this function.
            – Toby Speight
            Aug 24 at 6:42





            3




            3




            static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
            – Deduplicator
            Aug 24 at 11:24




            static state to avoid direct Iteration? Evil. Also, an int is potentially too small. And I'm not sure memoizing the solution is a good idea at all. Also, a using-declaration for a function used once, a typedef better replaced by more generous use of auto, and an aversion to empty lines? hm...
            – Deduplicator
            Aug 24 at 11:24

















             

            draft saved


            draft discarded















































             


            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f202307%2fproject-euler-problem-5-solution-in-c%23new-answer', 'question_page');

            );

            Post as a guest













































































            Popular posts from this blog

            How to check contact read email or not when send email to Individual?

            Displaying single band from multi-band raster using QGIS

            How many registers does an x86_64 CPU actually have?