Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Suggestions For Code Improvement  RSS feed

 
Lawrence King
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello everyone,

As a Java beginner, I decided to tackle the first question from Project Euler's problem set:

https://projecteuler.net/problem=1

In summary, it says:

If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.

Find the sum of all the multiples of 3 or 5 below 1000.

I've written some code to help me calculate this. See below:



By lowering the multipleLimit value to 10, I can verify that the code provides the correct solution. Something about the double use of the for-loop however makes me think there are much better ways to solve this.

I would be grateful if anyone could suggest avenues to pursue in which I could improve the general algorithm or the presented code itself.

My sincere thanks,
Lawrence
 
fred rosenberger
lowercase baba
Bartender
Posts: 12542
48
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What happens if you set your multiple limit to something like 20?  Does it still give the correct answer? (hint - i don't think it does).

 
Stefan Evans
Bartender
Posts: 1836
10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My suggestions:

You should be able to do this in one loop rather than two.

The upper bound on your loop goes all the way up to multiple limit.
That considers a whole lot of numbers that are never going to be valid.
Even with a limit of 10, only the first 3 values of 'i' are every going to be useful.  From i=4 upwards, multiplying by 3 gives you a value more than 10.
Can you come up with a better loop condition ?

 
Lawrence King
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you both - appreciate the quick responses.

Apologies, had to make a change to the code. If I print the following out to the console:



I get multiples < 20 for 5 and 3 respectively. Am I missing the error by printing out just after multipleSum += validMultiple?

I'm going to work on using one loop as suggested.

Many thanks,
Lawrence
 
Carey Brown
Bartender
Posts: 2996
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The if() on line 15 and line 22 is redundant with the 'while' portion of the for() loop.

The value of '15' is both a multiple of 3 and a multiple of 5, your code will add 15 twice to the sum.

edit: you changed the code out from under me.
 
Lawrence King
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Carey,

Yes, of course; thanks for pointing that out.

Back to the drawing board.

 
Liutauras Vilda
Marshal
Posts: 4641
318
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Anyway, how you know if number is multiple of 3 or 5? Think might % (modulus) operator would be handy here?
 
Liutauras Vilda
Marshal
Posts: 4641
318
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Actually your idea probably is even better, just requires small amendments. Somebody mentioned right path already
 
Stefan Evans
Bartender
Posts: 1836
10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So yes, just to clarify, if you run it with 20
Multiples of 3:  3,6,9,12,15,18   - add up to 63
Multiples of 5:  5, 10, 15           - add up to 30

As written originally your program would output 93 - because you count 15 as a multiple of both 3 and 5. 
Multiples of 3 and 5 < 20 :  3,5,6,9,10,12,15,18  --> 78


 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!