C++

Forum

 
no outputs

pnwadike99 (57)
had it working before I tried adding the sort and duplicate functions. now it just outputs a blank file and doesn't output anything to the console. any observations about where I went wrong? 

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
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
 
void introduction(string obj, string ins);  //user introduction
bool checkDuplicate(string tempName, string* rndName, int n); //check if name is a duplicate name
string changeNow(string s); //convert

//main program
int main()
{
  //Data
  string usersFile; //stores users file name
  const int MAX_RNDNAMES = 10; //list size
  int nrndNames = 0; //list capcacity
  string rndName[MAX_RNDNAMES]; //unchanged name array
  string tempName[MAX_RNDNAMES]; //changed name array
  int u; //counter
  string tempVar; //temp changed name sort slot
  string sempVar; //temp unchanged name sort slot
  string holdName; //current original unchanged name
  string runName; //name being changed and tested

  //user introduction
  introduction(objective, instructions);

  // get user file name
  getline(cin, usersFile);

  // open input file
  ifstream fin; //
  fin.open(usersFile.c_str()); //
  if (!fin.good()) throw "I/O error"; //
  
  // getnames
  for (int i = 0; i < MAX_RNDNAMES; i=i+1)
  {
    getline(fin, runName);
    holdName = runName;
    changeNow (runName);
    bool result = false;
    if (!fin.good())
    {
      break;
    }
    else if (checkDuplicate(runName, tempName, nrndNames))
    {
      if (result = false)
      {
        if (tempName[nrndNames].length() > 0)
        {
          rndName[nrndNames] = holdName;
          tempName[nrndNames] = runName;
          cout << rndName[nrndNames] <<endl << endl;
          nrndNames++;     
        }  
      }
    }
    else if (result = true)
    {
      i=i-1;
    }
    else if (tempName[nrndNames].length()==0 )
    {
      i=i-1;
    }  
  }
  
  //close input file
  fin.close();
  
  // open output file
  ofstream fout; //reserve "fout" for use
  fout.open ("friends.txt"); //open specifed file
  if (!fout.good()) throw "I/O error"; //if file isnt open output error to console

  //sort
  for (int j = 0; j < nrndNames; j++) //outer loopp tranverse
  {
    for (int o = j + 1; o < nrndNames; o++)
    { 
      if (tempName[j] > tempName[o])
      {
        tempVar = tempName[j];
        sempVar = rndName[j]; 
        tempName[j] = tempName[o];
        rndName[j] = rndName[o];
        tempName[o] = tempVar;
        rndName[o] = sempVar;
      }
    }
  }

  //output
  for (u = 0; u < nrndNames; u++)
  {
    cout << rndName[u] <<endl;
    cout << endl;
    fout << rndName[u] <<endl;
    fout << endl;
  }

  //close output file
  fout.close();

}//main

//convert
string changeNow(string s)
{
  for(int q = 0; q < s.length(); q++)
  {
    s[q]=toupper(s[q]);
  }
  return s;
} //convert

//check if user name is a duplicate name
bool checkDuplicate(string runName, string* tempName, int n)
{
  //data
  //tempName is the list of non-duplicate names
  //n is the size counter of the list
  //runName is the name being checked
  bool result = false; //false is not a duplicate, true is a duplicate guess
  int i; //loop index for traversing the list

  //check if duplicate name
  for(i=0; i < n; i = i + 1)
  {
    if (runName == tempName[i])
    {
      result = true;
    }//if
  }//for i
  return result;
}//checkDuplicate
salem c (3687)
Yes, you compile with maximum warnings.
1
2
3
4
5
6
7
8
9
10
11
12
 
$ g++ -Wall baz.cpp
baz.cpp: In function ‘int main()’:
baz.cpp:50:25: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
       if (result = false)
                         ^
baz.cpp:61:27: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     else if (result = true)
                           ^
baz.cpp: In function ‘std::__cxx11::string changeNow(std::__cxx11::string)’:
baz.cpp:113:20: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for(int q = 0; q < s.length(); q++)
                    ^


At least until the point your compiler will tell you about mixing up = and == in if statements.

Handy Andy (5051)
Hello pnwadike99,

I refer you back to message http://www.cplusplus.com/forum/beginner/271736/#msg1171606

You are still not providing a program that can be compiled and tested. There are no include files and I do know have any idea what you have used. My guesses do work to a point, but some of the code does not.

Also you have never provided the input file that you are using, so I do not know if the file I am using is like yours. Providing the input file allows others to use the same input that you are.

I did find one problem when you defined your variables.
1
2
3
4
5
6
7
8
9
10
11
12
13
 
//Data
const int MAX_RNDNAMES = 10; //list size

int nrndNames = 0; // list capcacity
//int u; // counter. Should be defined in the for loop as you did with the others.
//string usersFile; // stores users file name
std::string usersFile{ PATH + "5 Random Names.txt" };
string rndName[MAX_RNDNAMES]; // unchanged name array
string tempName[MAX_RNDNAMES]; // changed name array
string tempVar;  // temp changed name sort slot
string sempVar;  // temp unchanged name sort slot
string holdName; // current original unchanged name
string runName;  // name being changed and tested


"u" should be defined in the for loop instead of hanging around most of the program unused just taking up space.

I like to keep like types together, but in the end it makes no difference. IMO I think it is easier to read and keep track of the variables.

The line
1
2
 
// user introduction
introduction(objective, instructions);

The function is prototyped, but never defined in the code provides and the parameters are never defined. I have to comment this out just to get the program to compile. If you felt that this function was not necessary to the problem then comment it out do people will not be complaining about it being missing.

1
2
 
// get user file name
getline(cin, usersFile);

I guess that you provide the source code when someone other than you runs this program so they can try to figure out what the blank screen is waiting for.

I do not know where you learned this from, but
1
2
 
if (!fin.good())
    throw  "I/O error"; //

It does compile for me, but it does give a run time error when reading the string.

The next problem is that you throw an exception, but never deal with it. I do not know if your header files and compiler will cause the program to terminate by just throwing an exception. For me I can get it to throw an exception, but the program continues as if nothing is wrong.

Earlier in http://www.cplusplus.com/forum/beginner/271736/#msg1171606 I showed you the more often used method for checking if a file stream is open.

Given the code:
1
2
3
4
5
6
7
 
for (int i = 0; i < MAX_RNDNAMES; i = i + 1)
{
    getline(fin, runName);

    holdName = runName;

    changeNow(runName);

This works until the read sets the (eof) bit then the value of "runName" is undetermined. And this may also depend on what C++ standard you are compiling with. Then when you call the function what are you actually changing?

I believe this should work bettter for you:
for (int i = 0; i < MAX_RNDNAMES && getline(fin, runName); i = i + 1). This way when "i" == MAX or the read sets (eof) the for loop ends.

By the time you reach if (!fin.good()) it is to late. You could also write this as: if (!fin). It does the same job just shorter.

Then in this code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
    bool result = false;

    if (!fin.good())
    {
        break;
    }
    else if (checkDuplicate(runName, tempName, nrndNames))
    {
        if (result == false)
        {
            if (tempName[nrndNames].length() > 0)
            {
                rndName[nrndNames] = holdName;
                tempName[nrndNames] = runName;
                cout << rndName[nrndNames] << endl << endl;
                nrndNames++;
            }
        }
    }
    else if (result == true)
    {
        i = i - 1;
    }
    else if (tempName[nrndNames].length() == 0)

line 7 returns a value from the function that is used in the if condition.The first problem is that "runName" has a proper value to use, but "tempName" is an array of empty strings and "nrndNames" has a value of zero. When you get to the function the array "tempName" just contains empty strings and "nrndNames" is still zero, so the for loop never loops and you always return "false". So you never reach line 9, but if you did "result" is "false" because of the way you initialized the variable making the condition always evaluate to true.


Until you reach line 11 then "tempName[nrndNames].length() > 0" will always evaluate to false because value of array element zero is an empty string having a length of zero and zero is never greater then zero, so the condition evaluates to "false".

Line 20 will always evaluate to "false" because "false" is never equal to "true"

Also you never do anything to change the value of "result", so it is always "false".

The sort code does not work because "j" will put "o" 1 past the end of the array, but since "nrndNames" will have a value of zero the outer for loop never executes.

When you get to the output code you will find that the array "rndName" is still empty strings and since "nrndNames" will have a value of zero the for loop neve loops thus showing no output either to the screen or the file.

Some tips:
things like:
i = i + 1; can be shortened to i++; or i--;. The postfix means to add 1 after the variable is used and the prefix means add 1 before the variable is used.

An if statement like if (result == true) can be shortened to if (result). Since "result" is defines as a bool it can have only 2 values. Zeor meaning false or 1 meaning true.

You have lines of code like this: fin.open(usersFile.c_str()); // . If you do not have a comment leave off the "//". It caused people to stop and wonder if you left something out. Second if your compiler is using the 2011 standards or later the ".c_str()" is not needed. A "std::string" will work just fine. If you need ".c_str()" see if your IDE/compiler can be set to use the 2011 standards or consider updating the IDE/compiler. Using the 2014 standards would be a better choice.

Andy
pnwadike99 (57)
Wow that is a lot to unpack and I appreciate every bit of it thank you , I’m going to take some more time with it and see if I can clean it up with your advice , and my I’ll post the full program without anything cut after I play with it some more. Thank you guys
Handy Andy (5051)
Hello pnwadike99,

I hope you did not take what I said the wrong way and think that you have to rewrite the program.

Sometimes just knowing what is wrong or where it is going wrong is the first step to fixing it.

Turns out that the program will work the way that you have written it with a few changes.

Such as:
1
2
3
4
5
6
7
8
9
10
 
// open input file
ifstream fin(usersFile);
//fin.open(usersFile.c_str());

if (!fin)
{
    std::cerr << "\n     File \"" << usersFile << "\" did not open\n";

    return 1;
}

Your code: if (!fin.good()) throw "I/O error";. It would compile, but crash at runtime. The above code is more often used to achieve the same purpose. Line 2 allows you to define the file stream and open the file at the time the object is constructed. If your compiler is using the 2011, or later, standards a "std::string" will work. It does not need to be converted to a C string unless you are using pre-2011 standards.

While looking into "throw" to figure out what was going wrong I came to this: https://en.cppreference.com/w/cpp/language/except_spec Under Syntax notice the comments on the right. You will notice that something is used to deal with the exception that is being thrown. and the examples here https://en.cppreference.com/w/cpp/language/throw all use try/catch to deal with handling the exception.

This code may work for you and you may be including a header file I do not know about.

Jumping ahead a bit:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
bool checkDuplicate(string runName, string* tempName, int n)
{
    // data
    // tempName is the list of non-duplicate names
    // n is the size counter of the list
    // runName is the name being checked

    //check if duplicate name
    for (int i = 0; i < n; i = i + 1)
    {
        if (runName == tempName[i])
        {
            return true;
        }//if
    }//for i

    return false;
} // checkDuplicate

You will notice I did not define the variables the way that you did. They are not really needed. Define "i" in the for loop as it should be and instead of defining and setting "result" just return true or false. Most of the time it is better to use what you have available than to over code what you do not need.

In the for loop in "main" start with for (int i = 0; i < MAX_RNDNAMES && getline(fin, runName); i++) This way either side of AND (&&) can cause the for loop to fail. Plus you get the read out of the way before you continue with the for loop.

Inside the for loop:
1
2
3
 
holdName = runName;

changeNow(runName);

Now "runName" has a proper value for line 1 and for line 3 there is something to work with.

In the function that is called:
1
2
3
4
5
6
7
8
9
10
 
// convert
void changeNow(string& runName)
{
    for (size_t q = 0; q < runName.length(); q++)
    {
        runName[q] = toupper(runName[q]);
    }

    //return s;  // <--- Not needed now and was never captured when the function returns to "main".
} // convert

I changed this to a "void" function because there is no need to return anything. Passing the variable by reference is the better way to pass a "std::string" and any changes made in the function are reflected back in "main".

Although you have the ability to change the variable name in the function parameter. Most times it is best to use the same name the was sent to the function for clarity. Also defining the variable in the functions parameter is the same as defining it after the opening {. It becomes local to the function having function scope until the closing } looses scope and returns scope back to where it was called from.

Now comes the interesting part and a good example of use what you have.
bool result = checkDuplicate(runName, tempName, nrndNames);
. Originally you defined the variable and initialized it to "false", but never changed it anywhere. This way if the function returns "true" the if/else if statements will work properly.

The first if statement is not needed because it is handled in the for condition.

Instead of if (result = false) you can write that as if (!result). And if checking for true you can say if (result). Since the if condition is evaluated and converted into a bool type answer using a variable already defined as a "bool" means less work and you are using what you have.

The last else if (tempName[nrndNames].length() == 0) Tends to be a duplicate of what is above it and in all my testing I never reached this part. I believe it can be removed and should not cause any problem. At the very least comment it out and see what happens. You may have had an ides for its need that I have not seen yet.

The sort code is about 99% corret. The actual sorting works it is ate for loops that are a problem.
1
2
3
4
 
for (int j = 0; j < nrndNames; j++) //outer loop traverse
{
    for (int o = j + 1; o < nrndNames; o++)
    {

First off "i", "j" and "k" are the most often used variables in for loops. Sometimes you can still give them a better name.

The outer for loop goes from 0 to 9 which is fine, but the inner for loop goes from "j + 1 or 1 to 9. The problem is that when "j + 1" equals 10 the condition would be "o < nrndNames" or 10 < 10. The for loop fails before you check the last name in the array.

The hint here is that "o = J + 1", so "j" can not be greater than 8. This is based on "nrndNames" having a value of 10, but the principal still applies if "nrndNames" is less than 10.

With these changes and the file I used I got this output:

   Unsorted List
--------------------
Enrique Thomas
Londyn White
Maverick Chan
Susan Dixon
Ariella Leonard
Zaniyah Shah
Jaylah Nielsen
Rolando Roy
Devyn Cobb
Eliezer Mcpherson


    Sorted List
--------------------
Ariella Leonard
Devyn Cobb
Eliezer Mcpherson
Enrique Thomas
Jaylah Nielsen
Londyn White
Maverick Chan
Rolando Roy
Susan Dixon
Zaniyah Shah

The name "Susan Dixon" is in the input file 3 times to test for duplicates. Even tried it on a file containing 500 names and it only took the first 10, but there are no duplicates in this file.

I am not sure if I have mentioned this, but if I have not. A good source of reference is https://en.cppreference.com/w/ At least this is kept up to date where the reference section here is good, but needs updated.

Andy
pnwadike99 (57)
thank you so much andy my your advice was very helpful and helped me understand the concepts going into the program
Topic archived. No new replies allowed.