CS160 lab help

[heroicJ]

So for my CS140 class a UTK we need to write a function to convert a decimal to a binary. This is what I came up with (not we are provided with the shell, we are ONLY to edit the dec2bin function). I have spent hours trying to figure out what is wrong with this. Are there any suggestions? Thanks!(:

  #include <iostream>
#include <string>
#include <cmath>
#include <vector>

using namespace std;

bool isdecimal(string &input)
{
	// see bin2dec for description of how the below code works

	return input.find_first_not_of("0123456789") == string::npos;
}

void reverse(string &input) 
{
	int n = input.length();
	for (int k=0; k<n/2; k++) 
	  swap(input[k], input[n-1-k]);
}

string dec2bin(string &decstr)
{
	int digit, digitint, digitpow;
	int decval = 0;
	int n = decstr.length();
	int remainder;
	string binary;
	
	// convert string decstr to integer decval:
	// 1) reverse to obtain LSB-MSB digit ordery
	reverse(decstr);
	// 2) for each ascii digit, subtract '0' to 
	//    obtain integer value, multiply by 10^k
	for (int k = 0; k < n; k++){
		digit = decstr[k];
		digitint = digit - '0';
		digitpow = pow(digitint, k);
		decval += digitpow;
		}
		
		
	//    and continually add result to decval
	
	

	// apply successive-halving
	int i = 0;
	while (decval <= 1){
		remainder = decval % 2;
		decval = decval / 2;
		binary[i] = remainder;
		i++;
		}

	// reverse to restore MSB-LSB bit order
	reverse(binary); 
	return binary;
}

int main()
{
	string decstr;
	while (cin >> decstr) {
        if (isdecimal(decstr) == false)
		    cout << "input error\n";
		else
		    cout << dec2bin(decstr) << "\n";
    }
}

[LB]

Your dec2bin implementation is overly complex - what ever happened to good old modulus 2 divide by 2 over and over?

[heroicJ]

The comments were put in by the professor. He wants us to do it with the reversing and all that stuff. But for the successive halving I should continuously do mod 2 / 2?

[LB]

It looks like you're doing something close to right, but it just seems really convoluted. It's hard to tell with the improper indentation. Try cleaning it up a bit?

[heroicJ]

Is there a better way to convert the input string into a decimal value? I think that is where I'm getting my problems.

[Chervil]

Is there a better way to convert the input string into a decimal value?

I think there is. Though there are several errors in your code, I think that using the pow() function here is using a sledgehammer to crack a walnut. Typically this function will calculate the logarithm, multiply by the power, then take the antilog to give the result. That's a huge amount of number-crunching when all that is required is something as simple as a single multiplication by 10. There may also be rounding errors to add to the complications. So my recommendation is to try to do this without using pow(), and indeed without using anything from the <cmath> header.

One other comment, there are a couple of significant errors here:

    int i = 0;
    while (decval <= 1)
    {
        remainder = decval % 2;
        decval = decval / 2;
        binary[i] = remainder;
        i++;
    }

I think the loop condition should be while (decval > 0). Bearing in mind that binary is an empty string (its length is zero), accessing individual characters such as binary[i] will be an error, since the characters will be beyond the boundaries of the string. Instead, you should concatenate (add to the end) each character onto the end of the string. Also remember how you converted the original decimal character into an integer by subtracting '0'. You need to do the opposite here, the overall structure of the code has a certain symmetry about it.

[heroicJ]

I just realized the math error with while (decval <= 1). Silly mistake on my part!

As for binary would I concatenate it by making it a vector, or is there a better way? Forgive me for silly questions, I've only had a semester of C++.

[Chervil]

You can do what you need with the std::string. Probably the simplest way is to use the += operator though there are alternatives - the string class has lots of capabilities.

See for example:
http://www.cplusplus.com/reference/string/string/operator+=/
http://www.cplusplus.com/reference/string/string/push_back/
http://www.cplusplus.com/reference/string/string/append/

[heroicJ]

Thanks to the help from both of you my code is converting 1-9 correctly. So thanks for that, I appreciate it. However it does not accurately do 0, and 10+ all converts to 1010. I believe I know how to fix the situation with 0, but I just have one more question. I know I'm supposed to multiply by 10^k, but I just don't know why.

EDIT: Here's my code now. I fixed the issue with 0, however I didn't fix the issue with pow() yet.

#include <iostream>
#include <string>
#include <cmath>
#include <vector>

using namespace std;

bool isdecimal(string &input)
{
	// see bin2dec for description of how the below code works

	return input.find_first_not_of("0123456789") == string::npos;
}

void reverse(string &input) 
{
	int n = input.length();
	for (int k=0; k<n/2; k++) 
	  swap(input[k], input[n-1-k]);
}

string dec2bin(string &decstr)
{
	int digit, digitint, digitpow;
	int decval = 0;
	int n = (decstr.length());
	int remainder;
	string binary;
	
	// convert string decstr to integer decval:
	// 1) reverse to obtain LSB-MSB digit ordery
	reverse(decstr);
	// 2) for each ascii digit, subtract '0' to 
	//    obtain integer value, multiply by 10^k
	for (int k = 0; k < n; k++){
		digit = decstr[k];
		digitint = (digit - '0');
		decval = digitint * pow(10,k);
		}
		
		
	//    and continually add result to decval
	
	

	// apply successive-halving
	if (decval == 0){binary = "0";}
	else{
	while (decval > 0){
		remainder = decval % 2;
		decval = decval / 2;
		binary += (remainder + '0');}
		}

	// reverse to restore MSB-LSB bit order
	reverse(binary);
	return binary;
}

int main()
{
	string decstr;
	while (cin >> decstr) {
        if (isdecimal(decstr) == false)
		    cout << "input error\n";
		else
		    cout << dec2bin(decstr) << "\n";
    }
}

[Chervil]

but I just have one more question. I know I'm supposed to multiply by 10^k, but I just don't know why.

Sorry for the slow reply.

I think here, before trying to write any code, we need to look at the way ordinary numbers work. For example, the number 1933. That's one thousand, nine hundred and thirty-three. Let's break that down:
1 * 1000
9 * 100
3 * 10
3 * 1
Add all of those together and you get 1000 + 900 + 30 + 3 = 1933

So, in order to interpret the string "1933", it is broken down into digits - which you have done correctly, then multiply each digit by a weighting factor, according to which column it is in. Add together each of these weighted values.

Your code is very close, it just isn't adding all of these values together. Instead it just keeps the result for the leftmost digit. Look at line 38 in your code, it should be adding there.

As for my earlier comments regarding the use of pow(), here are some actual values output from one of my compilers; I added this code inside the body of the loop:

    long double tempow = pow(10,k);
    cout << "tempow = " << fixed << setprecision(18) << tempow << endl;

Output:

tempow = 1.000000000000000000
tempow = 10.000000000000000000
tempow = 99.999999999999999993
tempow = 1000.000000000000000167

(Your compiler may give different output).

Because the rest of the calculations are done using integers, anything after the decimal point is discarded, so that instead of 10²=100 we get 10²=99 which is clearly wrong. We could take steps to fix this by ensuring the value is correctly rounded instead of truncated, but since all of our values are integers, it's better to avoid these floating-point issues altogether by not using the pow() function here.

One suggestion is to start with a weighting factor something like int weighting = 1; Use that instead of pow(), and each time around the loop, multiply weighting by 10 and save that as its new value.

An alternative is to multiply decval by ten each time, this is exactly mirroring the binary conversion where decval is divided by 2 each time at line 51.

Please accept my apologies for taking what is a fairly simple topic and digressing into what might seem unnecessary complexity. I hope at least something here is useful at this point.

Online binary/decimal/hexadecimal converter:
http://www.mathsisfun.com/binary-decimal-hexadecimal-converter.html

[heroicJ]

I really appreciate your help. Using your suggestions I was able to get the code working! Thanks to both of you guys for your suggestions.