The book's implementation
Normally, asking for a review of code written by others is off-topic review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
I think that your version is much better! Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. Also, the
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
I think that your version is much better! Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. Also, the
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
I think that your version is much better! Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. Also, the
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
YourI think that your version is much better! Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. The Also, the
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. The
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
I think that your version is much better! Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. Also, the
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);
The book's implementation
Normally, asking for a review of code written by others is off-topic for Code Review. However, given the simplicity of the code, the context of your question (requesting a comparison with your own working implementation), and the fact that it's a book that you're learning from, I feel compelled to comment.
Your book seems to be promoting some questionable programming practices.
- Inconsistent whitespace: The indentation widths are not the same. The
ifcondition hasarr[i+1], which is written asarr[i + 1]in the statements below. - Misleading
forloop: The loop header makes it look like a simple loop in whichiis incremented byiwith each iteration. That's a lie! In fact,igets reset to-1whenever a swap occurs. What looks like a simple algorithm that makes one linear pass through the array is actually an O(SIZE2) algorithm. - Magic number: What is the significance of -1? It's a roundabout way to reset
ito 0, once thei++in the loop's epilogue is taken into account. - Weird loop limit: The last iteration through the loop, when
i = SIZE - 1, is guaranteed to have no effect, sinceif ((i < SIZE - 1) && ...)will always fail.
A less deceptive way to express the book's algorithm would be:
int i;
do
{
for (i = 0; i < SIZE - 1; i++)
{
if (arr[i] > arr[i + 1])
{
int swap = arr[i];
arr[i] = arr[i + 1];
arr[i + 1] = swap;
break; /* Start over at i = 0 */
}
}
} while (i < SIZE - 1);