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; 
}
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Sep 5, 2015 at 12:34
\$\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\$ 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\$ 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.

answered Sep 5, 2015 at 13:41
\$\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\$ Commented Sep 5, 2015 at 14:11
  • \$\begingroup\$ Passing -std=c++11 to the compiler fixed the above issue...thanks again! \$\endgroup\$ 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\$ 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).

answered Sep 5, 2015 at 12:46
\$\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;
 } 
}
answered Sep 5, 2015 at 16:59
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.