Thread: Works on PC, not on UNIX

I'm writing a program for homework that works fine on my XP box. I compiled the same program at school (UNIX), but it doesn't work there. I use Dev-C++ on the XP computer and g++ at school, so the compilers should be about the same.
The weird thing is that I can fix the error on the UNIX version just by accessing the variable in question (not doing anything with it, just using the variable name as a statement.)

The program is a scheduling program.
The program is too long to post, so I'll post the snippets that are involved so far. The problem seems to be in the import_course function, which should import a schedule from a file. It uses the add_course function, which can be called by the user as well to input a single course from the console. The first part of a course listing is the days that the class meets. This is where the problem lies. If is use the add function from the console, everything works fine. If I use the import function, it adds an F to every day listing:
import_course:

Code:

while (getline(inFile, oneCourse))
   {
      ++lineNum;
      stringstream cStr(oneCourse);
      vector<string> pCourse;
      pCourse.push_back("add");
      string piece;
      while (cStr >> piece)
         pCourse.push_back(piece);
//solution 1
      if (!add_course(sched, pCourse))
         cout << "(Import file: line " << lineNum << ")" << endl;
   }

add_course: (args[1] contains a string of characters representing the days the class meets.)

Code:

   if (!(args[1] == "o" || args[1] == "O"))
   {
      string week = "MTWRFS";
      unsigned int weekPos = 0, dayPos = 0;
      while ((dayPos < args[1].size()) && (weekPos < week.size()) )
      {
         if (toupper(args[1][dayPos]) == week[weekPos])
            ++dayPos;
         ++weekPos;
      }
      if (dayPos < args[1].size())
      {
         cout << "Error: add: days not valid. days can only include MTWRFS in chronological order" << endl;
         return false;
      }
   }

   DaysOfWeek nDays(args[1]);

I have found two 'solutions' to this problem.
If I replace //solution 1 with

Code:

pCourse[1]; //pCourse[1] contains the string that will be passed to the DaysOfWeek constructor

Also, if I comment out all of the above add_course function except for the constructor, everything works fine. I can't figure out what would be causing this error that it could be solved in both of these ways. The code in the add_course function doesn't change anything, and besides, it works when the add_course function is used without the import function calling it. However, without that section of code, everything works fine.
Since the extra 'F' only occurs when I call the import function (and then, only in UNIX), it would seem the error is there. The 'solution' is just to use the variable in a useless statement? It's easily fixable doing that, but I'm nervous that whatever's causing this could be causing problems elsewhere I don't know about yet.

As always, any help is most appreciated.

> ++dayPos;
> ++weekPos;
The first thing I would suggest is using extra braces to make your intentions much more obvious.

Second, try and reduce it to a simple example which still shows the problem. It would appear that the problem appears pretty early on, so cutting out all the code which runs after should be easy (make a backup first!).

Hopefully, you should end up with a small bit of code which you can post complete for others to look at and run.
Don't forget additional input files, or example command lines.

Code:

if (!(args[1] == "o" || args[1] == "O"))

Unless I miss my guess, args is defined as something like this:

Code:

char *args[]

in which case you'll need to use strcmp().

args[1] is a string, so == should be OK.
After trying to whittle down my program per Salem's advice, I think I may have found the problem. A DaysOfWeek object is really just an array of bool, day[6] (Sunday isn't included). One DaysOfWeek constructor takes a string, and if it finds an m, it sets day[0] = true, if a t is found, then day[2] = true, etc.
It's not just the F that's being carried around, it's anything that was previously constructed. I thought that since each was a new instance of DaysOfWeek, it would start fresh with all day[] members false, but preivous incarnations seem to be carrying over.

I'm in the middle of writing a totally different class/driver to demonstrate what's going on. I'll post it when I'm finished.

Originally Posted by dwks

Code:

if (!(args[1] == "o" || args[1] == "O"))

Unless I miss my guess, args is defined as something like this:

Code:

char *args[]

in which case you'll need to use strcmp().

That would be argv.

OK, here it is. I made an ingeniously named class, MyClass. It is a bool array, and when the constructor that takes a string is used, it should create a MyClass object, all false, except for the members of that array that share digits with the string entered. I'm not sure that made any sense, so here's the code:

Code:

#include <iostream>
#include <vector>
#include <string>
#include <fstream>

using namespace std;

class MyClass
{
  private:
    bool yes[5];

  public:
    MyClass();
    MyClass(const string&);
    void output(ostream&) const;
};


int main()
{
  vector<MyClass> collect;
  vector<string> input;
  ifstream inFile("imp2");

  string temp;
  while (inFile >> temp)
  {
    input.push_back(temp);
    MyClass MCtemp(temp);
    collect.push_back(MCtemp);
  }

  for (vector<MyClass>::size_type i = 0; i != collect.size(); ++i)
  {
    cout << input[i] << ": ";
    collect[i].output(cout);
    cout << endl;
  }

  cout << "Input one of your own...";
  string yourOwn;
  cin >> yourOwn;
  MyClass MCtemp(yourOwn);

  cout << yourOwn << ": ";
  MCtemp.output(cout);
  cout << endl;

  return 0;
}

MyClass::MyClass() {}

MyClass::MyClass(const string& str)
{
  for (string::size_type i = 0; i != str.size(); ++i)
  {
    if (str[i] == '0')
      yes[0] = true;
    if (str[i] == '1')
      yes[1] = true;
    if (str[i] == '2')
      yes[2] = true;
    if (str[i] == '3')
      yes[3] = true;
    if (str[i] == '4')
      yes[4] = true;
  } 
}
void MyClass::output(ostream& outs) const
{
  for (int i = 0; i != 5; ++i)
  {
    if (yes[i] == true)
      cout << i;
  }
}

If you run it with this file:
imp2:

Code:

3 2 1 0

You (or at least, I) would expect that there would be 4 MyClass objects in "collect," the first part of the output would be:

Code:

3:3
2:2
1:1
0:0

However, it looks like this:

Code:

3:3
2:23
1:123
0:0123

The user-derived input seems to work fine. Does the MyClass object temp not get destroyed with each new trip through the while loop?
I haven't tried it on XP yet, that will have to wait until I get home, unless someone else tries it. I fixed it (here and in the original program) by setting all elements of yes[] to false, but at the beginning of a constructor that will set them all to false anyway, should I really have to do that?

Two Words. Copy Constructor.

Code:

#include <iostream>
#include <vector>
#include <string>
#include <fstream>

using namespace std;

class MyClass
{
  private:
    bool yes[5];

  public:
    MyClass();
    MyClass(const string&);
    MyClass(const MyClass &copy)
    {
        //I am basically simulating the copy constructor made by the compiler
        cout<<"How'd we get here huh?"<<endl;
        for(size_t i = 0; i < 5; i++)
        {
            //What happens here ;)
            yes[i] = copy.yes[i];
        }
    }
    void output(ostream&) const;
};


int main()
{
  vector<MyClass> collect;
  vector<string> input;
  ifstream inFile("imp2.txt");

  string temp;
  while (inFile >> temp)
  {
    input.push_back(temp);    
    collect.push_back(MyClass(temp));
  }

  for (vector<MyClass>::size_type i = 0; i != collect.size(); ++i)
  {
    cout << input[i] << ": ";
    collect[i].output(cout);
    cout << endl;
  }

  cout << "Input one of your own...";
  string yourOwn;
  cin >> yourOwn;
  MyClass MCtemp(yourOwn);

  cout << yourOwn << ": ";
  MCtemp.output(cout);
  cout << endl;

  return 0;
}

MyClass::MyClass() {
    for(size_t i = 0; i < 5; i++)
    {
        yes[i] = false;
    }
}

MyClass::MyClass(const string& str)
{
  for (string::size_type i = 0; i < str.size(); ++i)
  {
    if (str[i] == '0')
      yes[0] = true;
    if (str[i] == '1')
      yes[1] = true;
    if (str[i] == '2')
      yes[2] = true;
    if (str[i] == '3')
      yes[3] = true;
    if (str[i] == '4')
      yes[4] = true;
  } 
}
void MyClass::output(ostream& outs) const
{
  for (int i = 0; i != 5; ++i)
  {
    if (yes[i] == true)
      cout << i;
  }
}

When you use a vector a copy constructor is called with or without your consent. So if you don't provide one C++ provides one for you which basically "shallow" copies all your data. And yes you need to initalize yes because if not it can be whatever it wants to be.

My bad. I thought it was copying the values but I guess not since I just tested it.

OK, so I can change my program's equivalent of yes[] to a vector, or set the whole array to false in the constructor. Thanks.

However, this brings up two questions:
1) Why did solution 1 (above) work?
2) Why did this only happen on UNIX and not XP?

And I still don't quite understand the vector/shallow copy thing. I create the object before I push it onto the vector, so how does that affect the object's creation? Or is it that it creates a new object using the default (copy? no parameters, anyway) constructor, then copy the data members from the one I'm pushing onto the vector into the new one the vector created for itself?

I googled copy constructors, and the syntax is a bit different than what prog-bman has in his changed code. If I understand correctly, the default constructor takes no parameters, and the copy constructor takes an object of the same class as a parameter, right?

Also, is the default constructor used at all when calling a different constructor? If not, why wouldn't I need to set all of the elements in the array to false in the MyClass::MyClass(const string&) constructor?

oops! I was looking at this:

Code:

MyClass::MyClass() {
    for(size_t i = 0; i < 5; i++)
    {
        yes[i] = false;
    }
}

I didn't see that he added the copy constructor up above.

It's all clear, now. Many thanks to both of you.