EvilZone
Programming and Scripting => C - C++ => : Recon December 29, 2013, 11:46:45 PM
-
Dear Evilzone community,
Looking to learn how to program, I decided to dive in headfirst, as I have seen recommended, and actually program something. To that end, I decided on a simple command-line program which would be capable of reversing an entire text file, reversing each line of the file independently (without changing their overall order), or reversing each word of the file independently (without changing the order of the words). Unfortunately, I have encountered difficulties with pointers as they pertain to passing strings (char arrays) which have stalled my progress. I've included my code below, and would much appreciate it if someone could help me correct my usage of pointers and/or variable scopes, or tell me whether I'm going about this in entirely the wrong manner.
#include <fstream>
#include <iostream>
using namespace std;
char* reverseString(char input[]);
int main(){
char *myStr = "Hiya!";
char *output = reverseString[*myStr];
cout << output;
}
char *reverseString(char *input) { // reverses the order of a string
int bufferSize = strlen(input);
char *output = new char[bufferSize];
for(int i = 0; i < bufferSize; i++) {
output[(bufferSize - 1) - i] = input[i];
}
return output;
}
-
You have to allocate memory. When you do:
char* temp = "Blah";
You are assigning the address temp points to to "Blah" in ASCII.
Try:
char* temp = new char[5];
*temp = "Ten!!";
cout << *temp;
-
You have to allocate memory. When you do:
char* temp = "Blah";
You are assigning the address temp points to to "Blah" in ASCII.
Try:
char* temp = new char[5];
*temp = "Ten!!";
cout << *temp;
I tried that, as you can see in the following code (which I modified slightly from how you put it, because the compiler squawked at me about the leading asterisk), but it didn't fix my problem. That particular portion of the code wasn't triggering an error before either (but maybe it would have at run time). The error is coming up at line 11 of the following code.
#include <fstream>
#include <iostream>
using namespace std;
char* reverseString(char input[]);
int main(){
char *myStr = new char[5];
myStr = "Hiya!";
char *output = reverseString[*myStr];
cout << output;
}
char *reverseString(char *input) { // reverses the order of a string
int bufferSize = strlen(input);
char *output = new char[bufferSize];
for(int i = 0; i < bufferSize; i++) {
output[(bufferSize - 1) - i] = input[i];
}
return output;
}
-
I tried that, as you can see in the following code (which I modified slightly from how you put it, because the compiler squawked at me about the leading asterisk), but it didn't fix my problem. That particular portion of the code wasn't triggering an error before either (but maybe it would have at run time). The error is coming up at line 11 of the following code.
#include <fstream>
#include <iostream>
using namespace std;
char* reverseString(char input[]);
int main(){
char *myStr = new char[5];
myStr = "Hiya!";
char *output = reverseString[*myStr];
cout << output;
}
char *reverseString(char *input) { // reverses the order of a string
int bufferSize = strlen(input);
char *output = new char[bufferSize];
for(int i = 0; i < bufferSize; i++) {
output[(bufferSize - 1) - i] = input[i];
}
return output;
}
get a better compiler if it did. It should be
*myStr = "Hiya!"
-
Or since you're using C++ use strings. But if you still want to use C strings use strncpy
-
get a better compiler if it did. It should be
*myStr = "Hiya!"
Is Visual Studio 2012 Express not the right thing to be using? I'm not sure what else to use on Windows, and I haven't yet made the move to Linux.
-
Is Visual Studio 2012 Express not the right thing to be using? I'm not sure what else to use on Windows, and I haven't yet made the move to Linux.
VS 2012 doesn't truly have full standards support unfortunately. It's missing all of C99 I believe and a chunk of the prior standard to that so it doesn't support those fun stuff. It does support strn* stuff though
-
I recommend CodeBlocks and GNU C++ (gcc). Great for little programs like this and bigger projects. But later you'll demand a better debugger and better code completion heh :P
-
So, I went back and re-wrote this to try to use strings. The problem is, I've tried five different approaches that I thought would be logical ways of appending a character to a string, and none of them work. They all cause run-time exceptions. Can someone set me straight as to how I might go about doing this? I've marked the error in the code below.
#include <string>
#include <iostream>
using namespace std;
string reverseString(string input);
int main(){
string myString = "Hiya!";
string myOtherString = reverseString(myString);
cout << myOtherString;
}
string reverseString(string input) {
string output;
for (int i = input.length - 1; i >= 0; i--) {
output.push_back(input.at(i)); //error on this line, various combinations of input[i], +=, append, and the functions shown all fail
}
}
-
So, I went back and re-wrote this to try to use strings. The problem is, I've tried five different approaches that I thought would be logical ways of appending a character to a string, and none of them work. They all cause run-time exceptions. Can someone set me straight as to how I might go about doing this? I've marked the error in the code below.
#include <string>
#include <iostream>
using namespace std;
string reverseString(string input);
int main(){
string myString = "Hiya!";
string myOtherString = reverseString(myString);
cout << myOtherString;
}
string reverseString(string input) {
string output;
for (int i = input.length - 1; i >= 0; i--) {
output.push_back(input.at(i)); //error on this line, various combinations of input[i], +=, append, and the functions shown all fail
}
}
Your error isn't where the compiler says it is. The error is actually on the line above it. length is a function not a public member, so call input.length() not input.length. This isn't C# or the fuck w/e uses it as a public member.
-
Thanks, bluechill! I got a basic version of my program working.
I have another question, though. How do I pause the program after accepting a getline() input? I've tried cin.get(), cin.ignore(), and getchar(), but none of them seems to work. The program just blitzes through when I hit enter. I've even tried stacking them, which seemed to fix my problems with the plain cin input I used for the menu, but isn't helping with the getline input (and besides, it makes using option two a pain because you have to hit enter twice to close the program). The command system("pause") doesn't seem to work either; it just hangs the program indefinitely, although that could be the fault of my firewall. What would you recommend?
Also, is there a less butt-ugly way of dealing with option 2 than stacking two cin.ignore() commands, or is that just a reality of life?
EDIT: I messed around a little and found some interesting behavior. I've included a second block of code below for which option 2 will automatically close after the sleep function. Any idea what is causing this?
EDIT 2: I've modified the code to include six cin.ignore() functions; this seems to fix the auto-closing issue, but now it's not outputting the text at all before it closes.
// when choosing menu option 1, program exits execution without pausing to display output
#include <string>
#include <iostream>
#include <sstream>
using namespace std;
string reverseString(string input);
string reverseManualInput();
int main(){
string myOutput;
short menuOption;
cout << "1. Reverse Manual Line Input\n";
cout << "2. Reverse Preset Test String\n";
cin >> menuOption;
switch (menuOption)
{
case 1:
myOutput = reverseManualInput();
break;
case 2:
myOutput = reverseString("Hello World!");
break;
default:
break;
}
cout << myOutput;
cin.ignore();
cin.ignore();
}
string reverseManualInput() {
string input;
string output;
getline(cin, input);
output = reverseString(input);
return output;
}
string reverseString(string input) {
string output;
for (int i = input.length() - 1; i >= 0; i--) {
output.push_back(input.at(i));
}
return output;
}
#include <string>
#include <iostream>
#include <sstream>
#include <stdlib.h>
#include <time.h>
#include <stdio.h>
#include <dos.h>
#include <Windows.h>
using namespace std;
string reverseString(string input);
string reverseManualInput();
int main(){
string myOutput;
short menuOption;
cout << "1. Reverse Manual Line Input\n";
cout << "2. Reverse Preset Test String\n";
cin >> menuOption;
switch (menuOption)
{
case 1:
myOutput = reverseManualInput();
break;
case 2:
myOutput = reverseString("Hello World!");
break;
default:
break;
}
cout << myOutput;
Sleep(4000); //seems to do this before accepting manual string output
cin.ignore();
}
string reverseManualInput() {
string input;
string output;
getline(cin, input);
output = reverseString(input);
return output;
}
string reverseString(string input) {
string output;
for (int i = input.length() - 1; i >= 0; i--) {
output.push_back(input.at(i));
}
return output;
}
-
Thanks, bluechill! I got a basic version of my program working.
I have another question, though. How do I pause the program after accepting a getline() input? I've tried cin.get(), cin.ignore(), and getchar(), but none of them seems to work. The program just blitzes through when I hit enter. I've even tried stacking them, which seemed to fix my problems with the plain cin input I used for the menu, but isn't helping with the getline input (and besides, it makes using option two a pain because you have to hit enter twice to close the program). The command system("pause") doesn't seem to work either; it just hangs the program indefinitely, although that could be the fault of my firewall. What would you recommend?
Also, is there a less butt-ugly way of dealing with option 2 than stacking two cin.ignore() commands, or is that just a reality of life?
EDIT: I messed around a little and found some interesting behavior. I've included a second block of code below for which option 2 will automatically close after the sleep function. Any idea what is causing this?
EDIT 2: I've modified the code to include six cin.ignore() functions; this seems to fix the auto-closing issue, but now it's not outputting the text at all before it closes.
// when choosing menu option 1, program exits execution without pausing to display output
#include <string>
#include <iostream>
#include <sstream>
using namespace std;
string reverseString(string input);
string reverseManualInput();
int main(){
string myOutput;
short menuOption;
cout << "1. Reverse Manual Line Input\n";
cout << "2. Reverse Preset Test String\n";
cin >> menuOption;
switch (menuOption)
{
case 1:
myOutput = reverseManualInput();
break;
case 2:
myOutput = reverseString("Hello World!");
break;
default:
break;
}
cout << myOutput;
cin.ignore();
cin.ignore();
}
string reverseManualInput() {
string input;
string output;
getline(cin, input);
output = reverseString(input);
return output;
}
string reverseString(string input) {
string output;
for (int i = input.length() - 1; i >= 0; i--) {
output.push_back(input.at(i));
}
return output;
}
#include <string>
#include <iostream>
#include <sstream>
#include <stdlib.h>
#include <time.h>
#include <stdio.h>
#include <dos.h>
#include <Windows.h>
using namespace std;
string reverseString(string input);
string reverseManualInput();
int main(){
string myOutput;
short menuOption;
cout << "1. Reverse Manual Line Input\n";
cout << "2. Reverse Preset Test String\n";
cin >> menuOption;
switch (menuOption)
{
case 1:
myOutput = reverseManualInput();
break;
case 2:
myOutput = reverseString("Hello World!");
break;
default:
break;
}
cout << myOutput;
Sleep(4000); //seems to do this before accepting manual string output
cin.ignore();
}
string reverseManualInput() {
string input;
string output;
getline(cin, input);
output = reverseString(input);
return output;
}
string reverseString(string input) {
string output;
for (int i = input.length() - 1; i >= 0; i--) {
output.push_back(input.at(i));
}
return output;
}
There is no "pausing" other than running a while loop or sleep() and the reason it's not outputting any text is because you need to flush the output buffer.
-
EDIT: Never mind. I got it.
-
As above, if you're using C++, why not use the string alias? Otherwise, in C, you can do this with recursion pretty easy too. Here's a method I wrote a while back:
#include <stdio.h>
#include <string.h>
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
char c[2] = {s[lower_index], s[upper_index]};
memcpy(s + lower_index, c + 1, 1);
memcpy(s + upper_index, c, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
void reverse_str(char *s)
{
reverse_str_recur(s, 0, strlen(s) - 1);
}
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
reverse_str(str);
printf("Reversed: %s\n", str);
return 0;
}
Just remember with C style strings you have a null terminator to work with.
-
As above, if you're using C++, why not use the string alias? Otherwise, in C, you can do this with recursion pretty easy too. Here's a method I wrote a while back:
#include <stdio.h>
#include <string.h>
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
char c[2] = {s[lower_index], s[upper_index]};
memcpy(s + lower_index, c + 1, 1);
memcpy(s + upper_index, c, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
void reverse_str(char *s)
{
reverse_str_recur(s, 0, strlen(s) - 1);
}
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
reverse_str(str);
printf("Reversed: %s\n", str);
return 0;
}
Just remember with C style strings you have a null terminator to work with.
Why call memcall every time while you can just use basic assignment s[index] = char
-
Although it may result in more readable code, I'm using GCC, and profiling shows that memcpy (not "memcall" I assume ;) ) is faster than direct assignment in this case. For larger strings, I'd rather not sacrifice performance in a handshake for slightly better readability.
GCC replaces an assignment operation by a call to any library function of its liking, but this doesn't guarantee that it is better necessarily, and nor would it be viable to prove or disprove, however, it would also be rather difficult with compiler optimization. memcpy is also designed to be the most efficient way of copy blocks of memory for C..
memcpy is also much safer in other cases that do not necessarily apply here, but for instance in structs where unaligned exceptions can occur with unaligned memory, memcpy will be much faster at the reduction in cost of the exception.
All in all, I see no reason for changing memcpy out with direct assignment here. There is no point.
If somebody doesn't like memcpy, then fine:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
*(s + lower_index) = s[upper_index];
*(s + upper_index) = s[lower_index];
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}But I'll stick with memcpy.
The other option would be memmove:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memmove(s + lower_index, s + upper_index, 1);
memmove(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
However I could've also reduced my original memcpy version with:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memcpy(s + lower_index, s + upper_index, 1);
memcpy(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
*EDIT: New version
#include <stdio.h>
#include <string.h>
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memcpy(s + lower_index, s + upper_index, 1);
memcpy(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
#define REVERSE_STRING(s) (reverse_str_recur(s, 0, strlen(s) - 1))
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
REVERSE_STRING(str);
printf("Reversed: %s\n", str);
}
-
Although it may result in more readable code, I'm using GCC, and profiling shows that memcpy (not "memcall" I assume ;) ) is faster than direct assignment in this case. For larger strings, I'd rather not sacrifice performance in a handshake for slightly better readability.
GCC replaces an assignment operation by a call to any library function of its liking, but this doesn't guarantee that it is better necessarily, and nor would it be viable to prove or disprove, however, it would also be rather difficult with compiler optimization. memcpy is also designed to be the most efficient way of copy blocks of memory for C..
memcpy is also much safer in other cases that do not necessarily apply here, but for instance in structs where unaligned exceptions can occur with unaligned memory, memcpy will be much faster at the reduction in cost of the exception.
All in all, I see no reason for changing memcpy out with direct assignment here. There is no point.
If somebody doesn't like memcpy, then fine:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
*(s + lower_index) = s[upper_index];
*(s + upper_index) = s[lower_index];
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}But I'll stick with memcpy.
The other option would be memmove:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memmove(s + lower_index, s + upper_index, 1);
memmove(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
However I could've also reduced my original memcpy version with:
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memcpy(s + lower_index, s + upper_index, 1);
memcpy(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
*EDIT: New version
#include <stdio.h>
#include <string.h>
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memcpy(s + lower_index, s + upper_index, 1);
memcpy(s + upper_index, s + lower_index, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
#define REVERSE_STRING(s) (reverse_str_recur(s, 0, strlen(s) - 1))
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
REVERSE_STRING(str);
printf("Reversed: %s\n", str);
}
Function overloading exists for a reason js. You don't need to use preprocessor macros for that. Yes it'll be slightly more inefficient but that's why there's inline which is more readable and gets the same result.
-
You just described the reason why I used a macro instead. I know I can do things at least 100 different ways for reversing a string here, I can't use them all at once though. I chose my specific implementation for my own reasons.
Btw - Overloading in the traditional sense is not supported in C; you can't just have two reverse_str() functions with different signatures and expect it to work based on the parameter types and count during the call. This isn't C++.
Compile this in C if you don't believe me:
#include <stdio.h>
#include <string.h>
void reverse_str(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
memcpy(s + lower_index, s + upper_index, 1);
memcpy(s + upper_index, s + lower_index, 1);
reverse_str(s, upper_index - 1, lower_index + 1);
}
void reverse_str(char *s)
{
reverse_str(s, 0, strlen(s) - 1);
}
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
reverse_str(str);
printf("Reversed: %s\n", str);
}
-
As above, if you're using C++, why not use the string alias? Otherwise, in C, you can do this with recursion pretty easy too. Here's a method I wrote a while back:
#include <stdio.h>
#include <string.h>
void reverse_str_recur(char *s, int lower_index, int upper_index)
{
if (upper_index <= lower_index) return;
char c[2] = {s[lower_index], s[upper_index]};
memcpy(s + lower_index, c + 1, 1);
memcpy(s + upper_index, c, 1);
reverse_str_recur(s, upper_index - 1, lower_index + 1);
}
void reverse_str(char *s)
{
reverse_str_recur(s, 0, strlen(s) - 1);
}
int main()
{
char str[] = "12345";
printf("Original: %s\n", str);
reverse_str(str);
printf("Reversed: %s\n", str);
return 0;
}
Just remember with C style strings you have a null terminator to work with.
That is quite a memory and instruction demanding function for a simple task like switching the value of two bytes.
12 bytes for each call + 8 bytes saving return and frame ptr and 4 additional bytes for "c" if we consider alignment.
In fact gcc allocates 0x28 bytes on the stack for each call with this function so reversing a 1024 string would cost 40 times as much memory space and numerous function calls, for something that could have cost 1024+9 bytes (2x4 for start and end pointer + 1 byte as temporary char storage).
edit: of course it would be only ~20 times more memory as the function takes n/2+1 calls to be reversed
-
You're essentially calling out this function as bad because of the extra variables used (fixed in my revisions if you read a few posts forward), and numerous function calls (explain how to avoid that with recursion? :S). As I mentioned above memcpy() was designed for efficiency, so to judge the function based on how many bytes it allocates on the stack is very odd. If you want to delve into micro-optimization then sure, but in most cases that is senseless.
Did you forget to read my edited version (or the rest of the posts within this thread)? ??? I also posted a version with memmove() too, but the idea here was the simplicity of a recursive method. Nobody chooses recursion for absolute optimization. Obviously you don't use recursion if you take into consideration the things you've mentioned and an iterative approach would be better, but I never claimed that a recursive approach would be the best solution, I simply posted a method demonstrating it. I wouldn't use this method on larger strings, but recursion was invented to replace complex iterative variations which achieve the same thing, and memcpy() was designed to be the most efficient way of copy blocks of memory in C.
You also don't *require* temp char storage, because chars can be treated as 8 bit integers in this case, enabling you to use bitwise operations. If you're idea of optimization is less memory usage then here is an example depicting the lack of a temp char to swap the values.
Example:
#include <stdio.h>
#include <string.h>
void revstr(char *s)
{
size_t len = strlen(s);
size_t limit = (size_t)(len / 2);
for (size_t i = 0; i < limit; ++i)
{
*(s + i) ^= *(s + len - i - 1);
*(s + len - i - 1) ^= *(s + i);
*(s + i) ^= *(s + len - i - 1);
}
}
int main()
{
char mystr[] = "testing";
revstr(mystr);
printf("%s\n", mystr);
getchar();
return 0;
}
This uses bitwise operations to swap the values from the first char to the last char (before the null terminator).
Optimization is not entirely about less memory however. Some of the fastest algorithms for sorting and generating primes use up lots of memory, but are optimized in their own context, based on the methodology being used.