Calling a char Function

Im know im doing some wrong here, And thats its probably something really stupid but here goes, Im Trying to call the fRead char Function. Its suppose to Open a file, Read and Save the contents to Text Variable (In this case Text) But when the Program runs its blank. My thought was the function is not being called. Here's the code. Im also guessing i do not need to put the length of each variable in so much the [] That is, But i had alot of compilation issues and that seemed to fix it.

#include <iostream>
#include <fstream>
#include <cstdlib>

using namespace std;
char File[50],Word[1000],Text[1000];
char Read(char File[50], char Word[1000], char Text[1000]);

char fRead(char File[50],char Word[1000],char Text[1000]){

ifstream Read;
cin.getline(File, 50);
Read.open(File);


if(!Read.is_open()){ //Checks if File is open
exit(EXIT_FAILURE); // Terminates Program if File is Open
}

Read >> Word[1000];
while(Read.good()){
Read >> Word[1000];
}

Text[1000] = Word[1000];
cout << Text[1000];
return Text[1000];



}
int main (){
char fRead();
cout << Text << endl;



return 0;
}
Last edited on
You should put your code in code tags. See this: http://cplusplus.com/articles/firedraco1/

Your problem is related to variable scope and name conflicts. You're creating multiple variables with the same name, so you're not changing the one you think you are.

You're also only reading an individual char and not a string... but I'll address that later..

Here's what's happening (see my comments):

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
#include <iostream>
#include <fstream>
#include <cstdlib>

using namespace std;

// ***********
//  This below line creates a variable called 'Text'
// which has global scope.  We'll call this Text_G (G for global)
char File[50],Word[1000],Text[1000];


char Read(char File[50], char Word[1000], char Text[1000]);

// ***********
//  This below line (the function parameter list)
// creates ANOTHER variable named 'Text', but this 'Text'
// is local to fRead.  We'll call this 'Text' Text_L
//
//  Now.. since there are two variables named Text, the compiler
// has to determine which one to use.  By default it uses the
// variable with the smallest scope.  In this case,
// that means that EVERYTHING IN THE BELOW FUNCTION is changing
// Text_L -- and leaves Text_G completely unchanged.
//

char fRead(char File[50],char Word[1000],char Text[1000]){

  ifstream Read;
  cin.getline(File, 50);
  Read.open(File);


  if(!Read.is_open()){
    exit(EXIT_FAILURE);
  }

  Read >> Word[1000];
  while(Read.good()){
    Read >> Word[1000];
  }

  Text[1000] = Word[1000];
  cout << Text[1000];
  return Text[1000];



}


int main (){
  char fRead();    // this line doesn't do anything, btw.
                   //  it just declares a new function prototype

  // ***********************
  //  here, Text_L no longer exists (it only existed in fRead
  //   but since this is no longer fRead, it's not around anymore)
  //
  //  so this below 'Text' actually prints Text_G
  cout << Text << endl;


return 0;
}



Here is some advise:

1) Don't use globals.

2) Avoid name conflicts. (Although I think you were unaware of the name conflicts here)

3) Dont' use char arrays. You're not understanding them. Even if you correct the above problem with the name conflict, that function won't do what you expect at all. Save yourself a headache and use std::string

4) To call a function you don't put the return type in front of it. Just put the function name, followed by the parameter list.


Here's how the above function probably SHOULD be:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
#include <iostream>
#include <fstream>
#include <string>    // string is MUCH easier to use.  Use it
#include <cstdlib>

using namespace std;
//char File[50],Word[1000],Text[1000];  // get rid of these globals
                                        //  globals are evil, and you should
                                        //  not get in the habit of using them

string fRead();  // fRead doesn't seem to need any parameters (but see notes below),
                 //  so the prototype should look like this
                 // Note I'm returning a STRING not a single CHAR


string fRead(){
  // make local variables File, Word, and Text as you need them
  //  in the function.  No need to make them all parameters.

  ifstream Read;
  string File;          // make File a string
  getline(cin,File);    // get it from cin
  Read.open(File);

  // I didn't change this, although I don't recommend exiting
  //  like this -- but whatever
  if(!Read.is_open()){
    exit(EXIT_FAILURE);
  }

  // now start reading words/lines and putting them in Text
  string Line;
  string Text;

  // I'd avoid >> as well.  Stick with getline
  //  Note:  I'm unsure exactly what you were trying to do here,
  //  so I'm guessing
  //  I'm assuming you meant to have 'Text' contain the entire file
  while(Read.good())
  {
    getline(Read,Line);
    Text += Line;          // put the line at the end of 'Text'
    Text += "\n";          // put a new line character at the end
  } // repeat until EOF or error

  return Text;  // return the text

}


int main (){
  // make another variable local to main
  string Text;

  // get the text from your fRead function
  Text = fRead();

  // and print it
  cout << Text << endl;

  return 0;
}


wshew! Hopefully that'll help point you in the right direction
Last edited on
Was just what i needed, I mis-understood the one tutorial witht he char arrays would seem. So i can i do text, But can i make a program that launches an .exe in the same sort of way? And something sends data to a host and synch? Its sockets for that isnt it?
Last edited on
@ Disch First off I would like to say that I am an amateur, and the only experience I have with any programming, besides UML diagrams and psuedocode from software engineering courses, are the fundamentals and basics of C++ and C#. I have heard in numerous places that global variables should always try to be avoided; first question is Why? Second question, the stream insertion operator does not read blank spaces which is why you recommend cin.getline() or are there less obvious reasons as well? Any and all opinions welcome, I am just trying to comprehend all this.
global variables should be avoided because every function sees it.
<< reads string until a white space is found so its impossible to read "hello world" that would only read "hello" use cin.getline() instead
Last edited on
I have heard in numerous places that global variables should always try to be avoided; first question is Why?


1) Their usage makes your program restrictive. IE: if you write a function to use globals, then your function will only work with those globals.

Say for example you're writing a game and you want a function to deal the player some damage:

1
2
3
4
5
6
int player_hp;

void DamagePlayer( int damage )
{
  player_hp -= damage;
}


Seems harmless, right?

The problem now is that 'DamagePlayer' only works with 'player_hp'. It can't damage anything else. What if there are multiple players? What if you want to damage an enemy? Would you have to write a seperate function for each player/enemy? Talk about a waste.

It's much better to minimize the scope of all variables so that they're generic enough to apply to anything within reason. A much better approach to the above would be to pass 'hp' as a parameter by reference:

1
2
3
4
void DealDamage( int& hp, int damage )
{
  hp -= damage;
}


Now you can deal damage to any player/enemy/whatever with the same function.


Another, more common example is with I/O:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
// not so good
void LogReport()
{
  // a function to give a report to the user
  //  let's say the report is in a string called 'Log':

  cout << Log;
}

/*
again -- the above seems harmless -- except it logs to cout (global)
What happens when you want to change your program to log to a file instead?
You'd have to go back and change the above
*/

// better
void LogReport(std::ostream& logout)
{
  logout << Log;
}
// now you can log to any ostream (a file, cout, cerr, whatever) 


------------------------------------------

2) They make the entire program assume responsibility for a variable's state. When everything in the program has access to a variable, then your code must assume that anything in the program can change that variable.

This is harder to come up with an example for... but trust me it's an organizational nightmare for moderate/large programs.

When you keep the scope as small as possible, it minimizes the responsibility that any single function has. It keeps code straightforward and simple.

When everything is global, functions usually end up taking on too much responsibility and do things they shouldn't. You wind up with spaghetti code where everything is confusing and all tangled up.


Second question, the stream insertion operator does not read blank spaces which is why you recommend cin.getline() or are there less obvious reasons as well?


To be honest, I know jack squat about istreams. I never use them.

I recommended getline here because he looked like he wanted to read the entire file, so it made more sense to read it as large of chunks as possible. IE: line at a time made more sense than word at a time.

I also remember hearing things about >> leaving stray characters in the buffer under some circumstances (but I think that's just if you try reading an int or something) -- but like I say I don't really know because I never do it.
Thank you for the insight, that really clarified a ton of what I learned in engineering; I never really knew why data abstraction was so heavily emphasized, just that programmers are encouraged to implement it. I have also read that parameter passing is the true power of programming, and judging from your example it is one of the major keys to compatibility. Thanks again for sharing.
Topic archived. No new replies allowed.