Decimal to binary program, is it good?

Question

2.57/5 (3 votes)

See more:

Hey everyone, I'm new to programming. I recently wrote a program to convert from decimal to binary in c++ and just want some better opinions if it is properly written. Excuse my ignorance in advance in c++ and other programming languages, just want to know if this code is decently written.

Well here it is:

-IN BINARY.H-

C++

class ntb {
public:
    void num_to_bin() {
        std::cin>>convte;
        recur(convte);
        go_back();
        return ;
    }

    void recur(int convte) {
        if(convte == 0) {
            return;
        }
        recur(convte/2);
        std::cout<<convte % 2;

    };

    void go_back() {
        while(convte > 0 ) {
            std::cout<<""<<std::endl;
            num_to_bin();
        };
    };

private:
    int convte;
};
ntb Binary_Convert;

-IN MAIN.CPP -

C++

#include <iostream>
#include "binaryC.h"

int main()
{
    Binary_Convert.num_to_bin();
}

Posted 16-Sep-14 15:17pm

wohstihs

Updated 16-Sep-14 15:59pm

George Jonsson

v2

Add a Solution

Comments

George Jonsson 16-Sep-14 22:00pm

First you need to learn how to format the code so it is readable.
I did it this time.

2 solutions

Solution 2

These are some tips to improve your code:

Descriptive Names: First of all, try to name your variable, class and method names descriptively. For instance, ntb isn't a very good name. Instead try to name the class some thing like "NumberToBinaryConverter".

Document your code: You should document your code to show people what your methods, functions do and why they exist. For example,

C++

/**
 *   This function accepts input from the console and calls the recur function.
 */
void num_to_bin() {
  std::cin>>convte;
  recur(convte);
  go_back();
  return ;
}

The comment above the method is the documentation associated with the method. There are these tools like doxygen that generate HTML files containing your code documentation.

Try to Follow a Naming Convention: I can see inconsistent naming style in your code. Use a style to name your variables. For example, I use this style,
1) PascalCase for class and function names.
2) camelCase for variable names.
As my style is more suitable for C#, I encourage you to follow the Google C++ style. You follow any style but be consistent with it. But when you work on old projects, follow the convention used there.

Use Spaces Wherever Possible: Use of enough space is not bad and can improve code readability. For example,

C++

std::cout << "Hello, world" << std::endl;

is better than

C++

std::cout << "Hello, world" << std::endl;

That being said, overwhelming spaces aren't that good. For example,

C++

window . DoSomeThing ( 10 , 12 , 30 , 20 ) ;

can hurt your eyes.

Brace Placement Style: As in the case of naming convention, you must be consistent with style of brace placement. Your first file uses Java style while your second file uses the ANSI style. You should have used any one of these styles.

I can also see the weird semicolon after the closing braces. While the C++ compiler allows the semicolon, that is not necessary.

Avoid Global Variables: Never use global variables. I see no reason for why you declared the global variable Binary_Convert. Instead, why not use a local variable,

C++

int main()
{
  ntb n;
  n.num_to_bin();
}

Now, the object n will get destroyed when it reaches the end of the scope. But with global variables the object will remain in memory until your program exits. See this page.

Declare a Constructor and a Destructor: Your class ntb should have a constructor that initializes the fields during construction. Something like this would be fine,

C++

ntb() {
  this->convte = 0;
}

A destructor should be declared if your class has pointer members to delete the data pointed by them. I don't know how familiar you are with pointers. If you're unfamiliar with pointers and references, you should learn them; they're so important to understand programming.

Posted 17-Sep-14 0:02am

Manikandan10

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

George Jonsson · Accepted Answer · 2014-09-16T16:24:00

Well, there are a few issues in your code, and this is without even trying if it works.

Use comments to explain what you are doing.

Variable naming

convte

Variable initialization

C++

int convte;
int convte = 0;

C++

for (int i; i < 5; i++)
{
    // In Release mode the code inside the loop was never executed.
    // In Debug mode it worked fine.
}

Circular function calls

num_to_bin

go_back

num_to_bin

Usage of UI specific methods

cin

num_to_bin()

C++

int main()
{
    int input = 0;
    std::cin >> input;
    Binary_Convert.num_to_bin(input);
}

cout

Spaces are not evil (in the right places)

C++

std::cout << convte % 2;

C++

std::cout<<convte % 2;

Bracket positioning { }

C++

void go_back() 
{
    while(convte > 0 )
    {
        std::cout << "" << std::endl;
        num_to_bin();
    };
};

C++

void go_back() {
    while(convte > 0 ) {
        std::cout<<""<<std::endl;
        num_to_bin();
    };
};

The code you have written can be done in one method.
I just changed your recursive method a bit

C++

int Int2BinaryString(int iNumber, char* szOutput, int iLength)
{
    // If the input string is not initialized return -1 as error code
    if (szOutput == NULL)
        return -1;

    // Break condition for the recursion
    if (iNumber == 0)
        return 0;

    Int2BinaryString(iNumber / 2, szOutput, iLength);

    // The position of the character to add is the same as the current length of the string
    int iPosition = strlen(szOutput);
    
    // If the position exceeds the length of szOutput, return the number of characters required.
    if (iPosition >= iLength)
        return iPosition + 1;

    // Set the current position to one or zero
    szOutput[iPosition] = (iNumber % 2 == 0) ? '0' : '1';
}

int main()
{
    char szBinary[10] = ""; // Needs to be initialized to empty string.
    int result = Int2BinaryString(7, szBinary, sizeof(szBinary));
    if (result == 0)
        std::cout << szBinary << std::endl;
    else
        std::cout << "Error code: " + result + std::endl;
}

As an end note.
No, your code is not very well written.
But if you want to learn, you can.