Assignment 2:
Common mistakes / deficiencies / style remarks
- m11
-
With a declaration like
int digits[10];
a possibility of more than 10 digits appearing was not checked.
Possible fix:
assuming the digits are computed in a while-loop
with loop condition (x!=0) and i is the
incrementing array index,
replace the loop header by the following:
while((x!=0)&&(i<10))
- m12
-
The program doesn't treat digits greater than 9 in a special way.
This leads to an ambiguity, for example:
let b=16, x=15. The output is 15. (A single-digit number in base 16).
But if x=21=16+5, the output is 15 again.
Possible fix:
use a separator (e.g. space or comma) in the case when b>10.
For example, the output section
of the code can look like this
printf("%d", digits[i]);
if (b>10) printf(" ", digits[i]); /* extra space */
Alternatively, it is possible to use letters as digits, which will
extend the b range up to 10+26=36. We can then require that b<=36
in this program and add the corresponding validation.
- m13
-
(Not a mistake, but a good suggestion). Instead of hardcoded
"magic numbers" like this:
int digits[100]; ...
for (i=0; i<100; i++) {...}
use symbolic constants. Tho possible ways to modify the above
fragment are:
/* First way, using a preprocessor definition (Sect.3.5)*/
#define _MAXDIGITS 100 /*
put before first use of _MAXDIGITS, usually before main() */
int digits[_MAXDIGITS]; ...
for (i=0; i<_MAXDIGITS; i++) {...}
/* Second way, using a name with `const' qualifier
(which could be mentioned on p.72 of Text, but is not.
See Kernighan-Ritchie, p.40) */
const int maxdigits=100; /*
belong to the declaration section in main */
int digits[maxdigits]; ...
for (i=0; i<maxdigits; i++) {...}
- m14
-
Discard.
(This label is due to Oleg's preference of the 2nd way (from the above two)
to introduce a symbolic constant. I agree with him in that the 2nd way
is better from today's style point of view; in particular, there is
nothing like preprocessor definitions in the Java language, which is
a descendant of C/C++. However, there is nothing wrong, even
with respect to style, in #define'ing symbolic names.)
- m15
- (Not really a mistake, still a good point).
A case is missed in a nested if-else.
Example:
printf("x="); scanf("%d",&x);
if (x>=0)
{
printf("b="); scanf("%d",&b);
if (b > 1)
{
/* do convertion */
} /* But what if x>=0 and b<=1 ?*/
}
else
{ /* print error message */ };
A so structured program doesn't perform the calulation
in the case when either x<0 or b<=1 (or both). In this sense
it is correct and "foolproof". However, it doesn't print an error
message, if x>0 and b<=1. Another "else" clause could be
inserted in place of the red comment. However, a better
structure, with more clearly separated validation and calculation, is this:
/* Input and validation */
printf("x="); scanf("%d",&x);
/* read x */
if (x<0) /* check x */
{
/* print error message, exit with return code 1 */ }
printf("b="); scanf("%d",&b);
/* read b */
if (b<=1) /* check b */
{
/* print error message, exit with return code 2 */ }
/* From this point on, we deal with valid input data */
/* do convertion, print results */
- m16
-
(This reflects Oleg's preference and is completely a matter of style.
No effect on mark.)
In the if-else construction
if (x<0)
{ ....
return(1);
}
else
{ .... }
the "else" keyword and its braces can be omitted.
Whenever the "if" condition is true,
the program simply terminates in the "if" clause and there is no danger
that the "if" branch will join the computation further down.
I don't have strong preferences about either of the two possibilities
and use both versions in my programs.