Skip to main content
Code Review

Return to Answer

Fixed typos and relayouted example rewrite.
Source Link
sineemore
sineemore
  • 1.8k
  • 11
  • 33

Instead of trying to get a performance increase with a faster solution let's writereview your program and try to make it a solid one.

Check this outexample:

var a int = 0 // 0
var b int     // default int value is 0, simularsimilar to above
var c = 0     // type is int, simularsimilar to above
d := 0        // same

The latter is short and nice, I suggest you to stick to it. var notation is required in package block (globals), when explicit type is needed and to overcome variable shading.

When overflow accuresoccurs the loop condition will be falsy and the loop will terminate.

To take it futherfurther it is better not to expect intint overflow and use MaxInt* constants from math package. Apart from guaranteed proper uintuint overflow there is nothing solid about intint, so it is better not to abuse itits behavior.

Let's take it futherfurther. After the inner loop you test the prime variable and print the current_prime number. So when you get current_prime % i == 0 as true you already know current_prime is not a prime number and you need to continue the outer loop. For such purpose Golang has labels that perfectly solve the task:

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. AlsoBetter, lets use uint64 directly to guaranty the maximum availiableavailable type. The nice thing about unsigned numbers is that Golang guarantees that they will properly overflow properly so we can check for zero value to terminate the loop.

Example rewrite

Here follows a possible rewrite of your original program:

Nice things about the rewriteThings accomplished:

  • only two variables
  • proper type istypes are used
  • overflow is handled
  • no hacks

Instead of trying to get a performance increase with a faster solution let's write a solid one.

Check this out:

var a int = 0 // 0
var b int     // default int value is 0, simular to above
var c = 0     // type is int, simular to above
d := 0        // same

The latter is short and nice, I suggest you to stick to it.

When overflow accures the loop condition will be falsy and the loop will terminate.

To take it futher it is better not to expect int overflow and use MaxInt* constants from math package. Apart from guaranteed proper uint overflow there is nothing solid about int, so it is better not to abuse it.

Let's take it futher. After the inner loop you test the prime variable and print the current_prime number. So when you get current_prime % i == 0 as true you already know current_prime is not a prime number and you need to continue the outer loop. For such purpose Golang has labels that perfectly solve the task:

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Also lets use uint64 directly to guaranty the maximum availiable type. The nice thing about unsigned numbers is that Golang guarantees that they will properly overflow so we can check for zero value to terminate the loop.

Nice things about the rewrite:

  • only two variables
  • proper type is used
  • overflow is handled

Instead of trying to get a performance increase with a faster solution let's review your program and try to make it a solid one.

Check this example:

var a int = 0 // 0
var b int     // default int value is 0, similar to above
var c = 0     // type is int, similar to above
d := 0        // same

The latter is short and nice, I suggest you to stick to it. var notation is required in package block (globals), when explicit type is needed and to overcome variable shading.

When overflow occurs the loop condition will be falsy and the loop will terminate.

To take it further it is better not to expect int overflow and use MaxInt* constants from math package. Apart from guaranteed proper uint overflow there is nothing solid about int, so it is better not to abuse its behavior.

Let's take it further. After the inner loop you test the prime variable and print the current_prime number. So when you get current_prime % i == 0 as true you already know current_prime is not a prime number and you need to continue the outer loop. For such purpose Golang has labels that perfectly solve the task:

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Better, lets use uint64 directly to guaranty the maximum available type. The nice thing about unsigned numbers is that Golang guarantees that they will overflow properly so we can check for zero value to terminate the loop.

Example rewrite

Here follows a possible rewrite of your original program:

Things accomplished:

  • proper types are used
  • overflow is handled
  • no hacks
Also I use a lot of also, by the way
Source Link
sineemore
sineemore
  • 1.8k
  • 11
  • 33

Also, asAs pointed by peterSO, the first prime number is 2, so we can start with it.

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Also lets use uint64 directly to guaranty the maximum availiable type. Also aThe nice thing about unsigned numbers is that Golang guarantees that they will properly overflow so we can check for zero value to terminate the loop.

Also, as pointed by peterSO, the first prime number is 2, so we can start with it.

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Also lets use uint64 directly to guaranty the maximum availiable type. Also a nice thing about unsigned numbers is that Golang guarantees that they will properly overflow so we can check for zero value to terminate the loop.

As pointed by peterSO, the first prime number is 2, so we can start with it.

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Also lets use uint64 directly to guaranty the maximum availiable type. The nice thing about unsigned numbers is that Golang guarantees that they will properly overflow so we can check for zero value to terminate the loop.

Source Link
sineemore
sineemore
  • 1.8k
  • 11
  • 33

Instead of trying to get a performance increase with a faster solution let's write a solid one.

The variables

var current_prime int
var prime bool
current_prime = 0

Check this out:

var a int = 0 // 0
var b int     // default int value is 0, simular to above
var c = 0     // type is int, simular to above
d := 0        // same

The latter is short and nice, I suggest you to stick to it.

You've declared the prime variable, but is first used only within the outer loop, so declare it there. Try to introduce new variables first time you need them.

The outer loop

for {
    // ...
}

Your for loop is infinite since there is no loop condition or a break statement. The current_prime will overflow and you'll start printing the same/wrong numbers.

Also, as pointed by peterSO, the first prime number is 2, so we can start with it.

for current_prime := 2; i > 0; i++ {
    // ...
}

When overflow accures the loop condition will be falsy and the loop will terminate.

To take it futher it is better not to expect int overflow and use MaxInt* constants from math package. Apart from guaranteed proper uint overflow there is nothing solid about int, so it is better not to abuse it.

The inner loop

for i := 2; i < current_prime; i++ {
    if current_prime % i == 0 {
        prime = false
        i = current_prime
    }
}

It took me a while to understand the purpose of i = current_prime line. No. This is not nice, use break to end the loop.

This one is much cleaner to me:

for i := 2; i < current_prime; i++ {
    if current_prime % i == 0 {
        prime = false
        break
    }
}

Let's take it futher. After the inner loop you test the prime variable and print the current_prime number. So when you get current_prime % i == 0 as true you already know current_prime is not a prime number and you need to continue the outer loop. For such purpose Golang has labels that perfectly solve the task:

outer:
    for prime := 2; prime > 0; prime++ {
        for i := 2; i < prime; i++ {
            if prime % i == 0 {
                continue outer
            }
        }
        fmt.Println(prime)
    }

A continue outer statement will break the inner loop and continue the outer one. When you place a label one line before the loop you may break and continue right to it. It helps escaping nested loops a lot.

Finally lets choose the appropriate types for our task. Since we don't need the signed numbers we can use uint instead of int. Also lets use uint64 directly to guaranty the maximum availiable type. Also a nice thing about unsigned numbers is that Golang guarantees that they will properly overflow so we can check for zero value to terminate the loop.

package main

import "fmt"

func main() {

outer:
    for prime := uint64(2); prime > 0; prime++ {
        for i := uint64(2); i < prime; i++ {
            if prime % i == 0 {
                continue outer
            }
        }
        fmt.Println(prime)
    }
}

Nice things about the rewrite:

  • only two variables
  • proper type is used
  • overflow is handled