Ytdyklly

I am looking to simplify my main() function. The nested if statements contain four lines of the same code, and I'm unsure of how to refactor and simplify it. I was attempting to use booleans, which seemed like a safe bet, but the logic gets skewed in the process.

#include <bits/stdc++.h>

#include <iostream>

#include <string>



int bits(unsigned long d) {

    int l;



    if (d <= 255) {

        l = 8;

    }

    else if (d > 255 && d <= 65535) {

        l = 16;

    }

    else if (d > 65535 && d <= 4294967295U) {

        l = 32;

    }

    else {

        std::cout << "32 bits (4294967295) or smaller." << std::endl;

    }



    std::cout << "Bits..................... " << l << std::endl;



    return l;

}



std::string convertToBinary(unsigned long decimal) {

    int l = bits(decimal);

    int i, r;

    std::string str;



    //  creates array

    for (i = 0; i < l; i++) {

        r = decimal % 2;

        decimal /= 2;

        str += std::to_string(r);

    }



    //  reverses array for binary value

    reverse(str.begin(), str.end());



    std::cout << "Binary Value............. " << str << std::endl;



    return str;

}



int main(void) {

    std::string input;

    std::cout << "------------ Input -----------" << std::endl;

    std::cin >> input;



    std::cout << "Input: " << input << std::endl;

    std::cout << "Length: " << input.length() << std::endl;



    int i, count = 0, j = 0;

    int length = input.length();

    int ascii;

    unsigned long nums;



    int numbers[33];

    std::string binaries;

    std::string chars;



    for (i = 0; i < length; i++) {

        //  if next input is digit

        if (isdigit(input[i])) {

            numbers[j] = input[i];

            // place digit in next decimal

            count = (numbers[j] - '0') + count * 10;

            //  if next input is char

            if (i == length - 1) {

                if (isdigit(input[length - 1])) {

                    nums = count;

                    std::cout << "------------ " << nums << " ------------" << std::endl;

                    binaries += convertToBinary(nums) + ' ';

                    count = 0;

                }

            }

            //  next number

            j++;

        }

        //  if next input is char

        else {

            chars[i] = input[i];

            ascii = (int) chars[i];

            //  if next input is digit

            if (count != 0) {

                nums = count;

                std::cout << "------------ " << nums << " ------------" << std::endl;

                binaries += convertToBinary(nums) + ' ';

                count = 0;

            }

            //  != end of letters

            std::cout << "------------ " << chars[i] << " ------------" << std::endl;

            std::cout << "ASCII.................... " << ascii << std::endl;

            binaries += convertToBinary(ascii) + ' ';

        }

    }



    std::cout << "n------- Binary Value of " << input << " -------" << std::endl;

    std::cout << binaries << std::endl;



    return 0;

}

The program takes an input, then outputs the corresponding number of bits, binary value, and if necessary, an ASCII value. If the user inputs "c357g98", then the program should get ASCII and binary values of "c", then retrieve the binary value of "357" (rather than "3", "5", then "7" individually), so any adjacent digits will be treated as a single int instead of separate integers. Each time I attempt to refactor, the program loses its ability to store the adjacent digits together as a single int.

In the first attempt at refactoring, I tried this, which takes the count and binary parameters:

void getNumbers(int c, std::string b) {

    std::cout << "------------ " << c << " ------------" << std::endl;

    b += convertToBinary(c) + ' ';

    c = 0;

}

Since the count is never returned, the input "c357g98" is treated as "c357g35798" and the numbers never reset.

So in my second attempt, I return the count variable as an int:

int getNumbers(int c, std::string b) {

    std::cout << "------------ " << c << " ------------" << std::endl;

    b += convertToBinary(c) + ' ';

    c = 0;



    return c;

}

And replace the four lines of code with a reference to the function as:

count = getNumbers(count, binaries);

This method converts each value correctly, but does not store the final binary values of the digits, only the characters (since the reference to the string binaries was moved and not returned).

I've also done away with the nums variable entirely.

I'm coming to terms with the idea that my entire program may need heavily refactored in order for the logic to be simplified, but I was hoping to start with the messy main() function for now.

The first 3 lines in main can be turned into a function:

std::string getInput()

{

    std::string input;

    std::cout << "------------ Input -----------" << std::endl;

    std::cin >> input;

    return input;

}



int main()

    std::string input = getInput();

There are several portions of main that could become functions.

Declare Variable when needed

In the function convertToBinary the integer variables i and r are declared at the beginning of the function. They are only needed within the for loop.

    for (int i = 0; i < l; i++) {

        int r = decimal % 2;

        decimal /= 2;

        str += std::to_string(r);

    }

This is also true in the main function for i and other variables.

The contents of the then clause and the else could both be moved into functions.

Does the inner if statement (if (i == length - 1)) need to be within the for loop or could it be executed after the for loop is done? You might want to think about loop invariants.

Reduce Complexity When You Can

In the function bits checking from the top down might make reading easier because the top limit isn't needed

    if (d > 65535) {

        l = 32;

    } else if (d > 255) {

        l = 16;

    } else {

        l = 8;

    }