9
\$\begingroup\$

I have written VERY simple code that converts a string to a character array and then displays the ASCII value of each character. Let me know if this is the most effective/safe way of doing what I described above.

#include <iostream>


using namespace std;


int main(int argc, char *argv[]){
    //Declare Variables.
    string input;
    int size;

    //Display message to user asking them to enter a string or character.
    cout << "What character/string do you need the ascii value(s) of? " << endl;

    //Assign user input to string input.
    cin >> input;   

    //Get string length of user input.
    size = input.length();

    //Copy the string into a character array.
    const char * chars = input.c_str();

    //Itterate through the character array using the string size.
    for(int i = 0; i < size; i++){
            //Return Ascii values of each character.
            cout << "The value of character " << chars[i] << " is " << (int)chars[i] << endl;
    }

    return 0;       
}
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! I cleaned up some of the noise in your question so we can focus on the code. It's great to have you here! \$\endgroup\$
    – RubberDuck
    Commented Sep 5, 2015 at 12:38
  • 1
    \$\begingroup\$ gets.chomp.chars.each {|char| print "The ASCII value of #{char} is #{char.ord}\n"} Is the same as your program but in Ruby, just for comparison... \$\endgroup\$
    – Caridorc
    Commented Sep 5, 2015 at 14:50

3 Answers 3

12
\$\begingroup\$
int main(int argc, char *argv[]){
    //Declare Variables.
    string input;
    int size;

Don't declare variables until you actually need them, and don't declare variables you don't need. In this case, you're not using the arguments passed into main, either don't name them, or use the version of main that takes no arguments:

int main() { /* space or newline between paren and curly is usual */

cin >> input;

I'm not sure this is what you want. If the user enters a "sentence" with spaces in it, you'll only get what they typed up to the first space (not included). If you want the whole line, use std::getline.

std::string input;
std::getline(std::cin, input);

//Copy the string into a character array.
const char * chars = input.c_str();

This comment is dangerously wrong. You don't get a copy at all, you get a pointer to std::string's internals. The string data is not copied, and the pointer returned will become invalid as soon as input's lifetime ends.

std::string's length() member returns an unsigned quantity. While there's no problem in this case assigning it to an int, the correct type to use is:

std::string::size_type size = input.length();

or more conveniently:

auto size = input.length();

(And you'll need to use an unsigned quantity for the loop counter too, to avoid warnings.)


A different way of doing your loop would be to use the range-based variant. It works for std::string.

for (auto c: input) {
  std::cout << "Char \"" << c << "\" has value " << static_cast<int>(c)
            << std::endl;
}

This avoid getting the size altogether.

\$\endgroup\$
3
  • \$\begingroup\$ Very constructive answer thank you very much; Plus point for having the same name as me -- i did think of using getline i also tried using auto c for my loop however it complained "range-based ‘for’ loops are not allowed in C++98 mode" \$\endgroup\$
    – IndieBlock
    Commented Sep 5, 2015 at 14:11
  • \$\begingroup\$ Passing -std=c++11 to the compiler fixed the above issue...thanks again! \$\endgroup\$
    – IndieBlock
    Commented Sep 5, 2015 at 14:14
  • 1
    \$\begingroup\$ Yes, should have mentioned that sorry. The range-based for and auto were introduced in C++11. Good flags if you're using g++ or clang++: -Wall -Wextra -pedantic, with -std=c++XY for the standard you're targeting. \$\endgroup\$
    – Mat
    Commented Sep 5, 2015 at 14:20
11
\$\begingroup\$

Comments

Code should be self commenting. Your comments are superfluous.

//Declare Variables.
string input;
int size;

Namespaces

using namespace std; is considered bad practice. Short code is not a requirement in C++, clear code is preferred.

Return

return 0; is a legacy from C. In C++, it's no longer required to write this manually. The compiler will take care of returning 'normal' if no errors where thrown or other returns (like -1) are encountered.

Input validation

You expect the user to input a positive number at cin >> input;. However, there is nothing in your program checking whether the input is valid. A string will take pretty much anything you throw at it, so it will always accept it's new value (unlike int for example). However, you usually do not want all input to be accepted. What should happen in case of an empty line for example? And how does your program respond to special characters like , ƒ, and (notice the latter is actually one character, not two !).

You'll either want to validate or sanitize your input data (or both).

\$\endgroup\$
0
7
\$\begingroup\$

Don't use using namespace std;, it is considered bad practice. Also, int main() is fine if you aren't going to be using the variables. The comments are useless, because anyone can see it from the code. Use std::getline(std::cin,input) instead of cin >> input, because std::cin >> will truncate the string at the first space. You don't need to convert the string to an array of chars, instead just use input[i] to access the ith element. return 0 is not needed because it is automatically inserted at the end by the compiler. Also, I believe that you need to #include <string> for this.

#include <iostream>
#include <string>
int main(){
    std::string input;
    int size;

    std::cout << "What character/string do you need the ascii value(s) of? " << std::endl;

    std::getline(std::cin,input); 
    size = input.length();    
    for(int i = 0; i < size; i++){
        //Return Ascii values of each character.
        std::cout << "The value of character " << input[i] << " is " << (int)input[i] << std::endl;
    }      
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.