StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>Your function <code>is_unique_and_insertable</code> presumably should check whether an equivalent triple (the same numbers in different order) is already present in the list or the new triple is a multiple (modulo permutations) of a triple in the list. But it only compares the new triple to the first list element, there is no looping statement or recursion in the function.</p> <pre class="lang-c++ prettyprint-override"><code>int is_unique_and_insertable(ppt_ptr curr_ptr, int v1, int v2, int v3) { if(curr_ptr == NULL) { //printf("List is empty.\n\n"); } else { if(curr_ptr != NULL) { //printf("%d %d %d\n",curr_ptr->v1,curr_ptr->v2,curr_ptr->v3); int u1 = curr_ptr->v1; int u2 = curr_ptr->v2; int u3 = curr_ptr->v3; if( (different_triples(v1,v2,v3,u1,u2,u3)) && (!are_exact_multiples(v1,v2,v3,u1,u2,u3) ) ) { printf("%d %d %d\n",curr_ptr->v1,curr_ptr->v2,curr_ptr->v3); return 1; } //printf("yoyoyo"); curr_ptr = curr_ptr->next; } } return 0; } </code></pre> <p>You would get it to compare to more than just the first element if you used a <code>while(curr_ptr != NULL)</code>. However, it would still have the wrong logic, it would return true (1) as soon as it finds a different triple the new one is not a multiple of.</p> <p>The logic must be the other way round, if an equivalent triple (or a triple the new one is a multiple of) is encountered, then you return false (0), only if the entire list is traversed without encountering such a triple should you return true:</p> <pre class="lang-c++ prettyprint-override"><code>int is_unique_and_insertable(ppt_ptr curr_ptr, int v1, int v2, int v3) { while(curr_ptr != NULL) { int u1 = curr_ptr->v1; int u2 = curr_ptr->v2; int u3 = curr_ptr->v3; if (!different_triples(v1, v2, v3, u1, u2, u3) || are_exact_multiples(v1, v2, v3, u1, u2, u3)) { return 0; } curr_ptr = curr_ptr->next; } return 1; } </code></pre> <p>That takes you closer to a correct program, but your <code>are_exact_multiples</code> function is faulty, it would declare <code>(15, 36, 39)</code> to be a multiple of <code>(3, 4, 5)</code>, although it isn't.</p> <p>You would get a much simpler and easier to get right program if you only considered triples <code>(a, b, c)</code> with <code>a <= b <= c</code> (actually, <code>a < b < c</code>, since a Pythagorean triple can't have two equal components).</p> <p>You said you would treat efficiency later, but please do that soon, your <code>is_prime</code> function is painfully inefficient. You should stop as soon as you found the first nontrivial divisor, and you can stop when you've reached the square root:</p> <pre class="lang-c++ prettyprint-override"><code>int is_prime(int n) { if (n < 2) return 0; if (n%2 == 0) return n == 2; for(int d = 3; d*d <= n; d += 2) { if (n%d == 0) return 0; } return 1; } </code></pre>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload