My program converts decimal numbers from to binary and hexadecimal. I
am having trouble with my output which is supposed to be in a certain
format. Binary needs to be in the format of
XXXX XXXX for numbers 0 to 255. What am doing wrong or not doing with
my output?
Lot's of things
.
I presume that this is more or less homework, or an exercise for
you to learn by. Otherwise, presumably, you would use the hex
manipulator to output the hex. (Regrettably, there is no binary
manipulator.)
#include <iostream>
#include <iomanip>
Something to avoid in production code, of course.
void binary(int);
void hexdecimal(int);
Just wondering, do you want the values to really be signed?
(Not that it matters for your test values.)
int main()
{
int array[256];
int hex[256];
int i;
What are these declarations for, here? You never use hex or i
at all, and you only write to array, you never read it.
cout << "DECIMAL " << " BINARY " << " HEXADECIMAL " <<
" BCD" << endl;
That could be one single string, of course.
cout << endl;
for(int i=0;i<256;i++)
{
array = i;
cout << i;
binary(i);
hexdecimal(i);
cout<< endl;
}
return 0;
}
void binary(int i)
{
int rem;
if(i<=1)
{
cout << setw(8) << i;
return;
}
rem = i % 2;
binary(i >> 1);
cout << rem;
}
Several general comments:
-- A function should never output to std::cout. Pass it an
std:stream& as a parameter. Or have it return an
std::string.
-- You also don't take into consideration the fact that i might
be negative. If this is a pre-condition, then you should
have at least an assert; otherwise, you need to handle it.
-- Recursion is not particularly the best solution here;
although it can definitely be made to work, it handles
formatting very awkwardly. (It is a reasonable solution if
you generate non-formatted output, and then use a separate
routine to copy-format later. More on that below.)
-- You don't need the variable rem, either. Just output i % 2.
-- You definitely don't want the setw in the output in the if.
Each iteration outputs exactly one character. If the rest
worked (i.e. the condition in the if were correct), then
you'd have already output 7 digits before entering the if.
-- You're end condition doesn't take into account the number of
digits generated, at all.
-- You've no logic what so ever to generate the space in the
middle of the output.
If you're going to use recursion, and also want to format when
doing so, then you'll need information concerning the number of
digits and the format to be passed in each recursion. A
recursive solution which does both rapidly becomes painful.
void hexdecimal(int i)
{
char buffer[33];
itoa(i,buffer,16);
cout << " ";
cout << setw(12) << buffer;
}
The first two comments for binary also apply here. In addition,
there is no function itoa in C++.
Note that converting an int (or an unsigned int) to any base is
roughly the same. Without formatting, you can use either
recursion or iteration:
Iteration:
std::string
toString( unsigned int i, int base )
{
assert( base >= 2 && base <= 36 ) ;
std::string result ;
while ( i != 0 ) {
result += "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"[ i %
base ] ;
i /= base ;
}
std::reverse( result.begin(), result.end() ) ;
return result ;
}
Recursion:
std::string
toString( unsigned int i, int base )
{
std::string result ;
if ( i >= base ) {
result = toString( i / base, base ) ;
}
result += "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"[ i % base ] ;
return result ;
}
(Note that I've used unsigned int as an argument, to avoid the
issue of the sign.)
To ensure proper formatting, there are two solutions: do it
during the conversion, or use a copy-format after conversion.
To do it during converion, you have to pass the formatting
information to the "toString" function, above. Something like:
struct Spacing
{
int grouping ;
char sepChar ;
} ;
std::string
toString( unsigned int i, int base, Spacing const& fmt )
{
assert( base >= 2 && base <= 36 ) ;
std::string result ;
int generated = 0 ;
while ( i != 0 ) {
result += "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"[ i %
base ] ;
++ generated ;
if ( generated % fmt.grouping == 0 ) {
result += fmt.sepChar ;
}
i /= base ;
}
std::reverse( result.begin(), result.end() ) ;
return result ;
}
You can work out something similar for the recursive version if
you want, but note that the above uses an additional state
variable, which must be passed during recursion.
The alternative solution would be to use some sort of
copy-format function; a function which takes a format string
(e.g. something like the PICTURE clause in Cobol, or the format
string in a PRINT USING in Basic), and copies the unformatted
converted text using that, e.g.:
std::string
copyFormat( std::string const& format, std::string const& number )
{
std::string result ;
typedef std::string::const_reverse_iterator
SrcIter ;
SrcIter src = number.rbegin() ;
for ( SrcIter fmt = format.rbegin() ;
fmt != format.rend() ;
++ fmt ) {
switch ( *fmt ) {
case '#' :
result.push_back( src == number.rend()
? ' '
: *src ++ ) ;
break ;
case '9' :
result.push_back( src == number.rend()
? '0'
: *src ++ ) ;
break ;
default:
result.push_back( *fmt ) ;
break ;
}
}
std::reverse( result.begin(), result.end() ) ;
return result ;
}
In this case, you're output in the loop in main would be
something like:
std::cout << std::setw( 12 ) << i
<< copyFormat( " 9999 9999", toString( i, 2 ) )
<< copyFormat( " ####", toString( i, 16 ) )
<< std::endl ;
(Note that all of the above code is just off the top of my head;
none of it has been tested, or even compiled. So there are
probably a few errors. But it should give you some ideas to
start with.)