tags:

views:

97

answers:

5

Here is the header for a class I started:

#ifndef CANVAS_
#define CANVAS_

#include <iostream>
#include <iomanip>
#include <string>
#include <stack>

class Canvas
{
  public:

  Canvas();
  void Paint(int R, int C, char Color);
  const int Nrow;
  const int Ncol;
  string Title;
  int image[][100];
  stack<int> path;
  struct PixelCoordinates
   {
    unsigned int r;
    unsigned int c;
   } position;
   Canvas operator<< (const Canvas& One );
  Canvas operator>>( Canvas& One );
};

/*-----------------------------------------------------------------------------
   Name:      operator<<
   Purpose:   Put a Canvas into an output stream
-----------------------------------------------------------------------------*/
ostream& operator<<( ostream& Out, const Canvas& One )
{
  Out << One.Title << endl;
  Out << "Rows: " << One.Nrow << "    Columns: " << One.Ncol << endl;
   int i,j;
   for( i=0; i<One.Nrow; i++)
   {
     cout<<"\n\n\n";
     cout<< " COLUMN\n";
     cout<< " 1 2 3";
     for(i=0;i<One.Nrow;i++)
     {
       cout<<"\nROW "<<i+1;
          for(j=0;j<One.Ncol;j++) cout<< One.image[i][j];
     }
   }

  return Out;
}

/*-----------------------------------------------------------------------------
   Name:      operator>>
   Purpose:   Get a Canvas from an input stream
-----------------------------------------------------------------------------*/

istream& operator>>( istream& In, Canvas& One )
{
//  string Line;
//  int Place = 0;

//  {
//    In >> Line;

//    if (In.good())
//    {
//      One.image[Place][0] = Line;
//      Place++;
//    }
//  return In;

#endif

Here is my implementation file for class Canvas:

using namespace std;
#include <iostream>
#include <iomanip>
#include <string>
#include <stack>
#include "proj05.canvas.h"

//----------------Constructor----------------//

Canvas::Canvas()
{
  Title = "";
  Nrow = 0;
  Ncol = 0;
  image[][100] = {};
  position.r = 0;
  position.c = 0;
}

//-------------------Paint------------------//
void Canvas::Paint(int R, int C, char Color)
{
  cout << "Paint to be implemented" << endl;
}

And the errors I'm getting are these:

proj05.canvas.cpp: In function 'std::istream& operator>>(std::istream&, Canvas&)':
proj05.canvas.cpp:11: error: expected `;' before '{' token
proj05.canvas.cpp:24: error: expected `}' at end of input

From my limited experience, they look like simple syntax errors but for the life of me, I cannot see what I am missing. I know putting a ; at the end of Canvas::Canvas() is wrong but that seems to be what it expects. Could someone please clarify for me?

(Also, I know much of the code for the << and >> operator definitions look terrible, but unless that is the specific reason for the error please do not address it. This is a draft :) )

+5  A: 

You're missing a } for istream& operator>>( istream& In, Canvas& One ) in the header.

The data member int image[][100]; is also invalid, as is image[][100] = {}; in the ctor.

Your implementation (.cpp) files should #include their corresponding header first. This is a simple way to ensure the header is self-contained. In this case, it would lead to syntax errors in standard library headers, which quickly points out to you the problem is in the header (since that is what will be before the stdlib #includes).

Roger Pate
+2  A: 

You do not have a closing } at the end of operator >>

Noah Roberts
+3  A: 

You are missing a closing } for your operator>> body. Also, this is not correct:

using namespace std;
#include <iostream>

The order has to be the other way around. GCC's behavior isn't conforming: std must not be visible if no standard header is included yet. So on the next more conformant compiler, it may fail hard.

Also, in the header you should write std::istream instead of just istream (same for ostream, etc). In your current code, your header relies on its users to have typen using namespace std; before including it, which is pretty ugly from a design point of view, since it makes the headers dependent on its users in a non-obvious way (normally, it should be the other way around).

Johannes Schaub - litb
Beat you by a full minute this time, you're slacking!
Roger Pate
@Roger, shame on me!
Johannes Schaub - litb
A: 

Others have answered your question, but here are a couple of extra points:

If your definitions of the insertion and extraction operators aren't templated then you should move them to your implementation file and merely declare them in the header file. This will allow you to replace the line

#include <iostream>

with the line

#include <iosfwd>

The latter will only include declarations of the IO streams, not the entire definitions. This in turn will translate to a faster compilation time of every compilation unit that includes that header.

On the other hand, you could as well choose to leave those functions in the header file but template them.

template <typename Ch,typename Tr>
std::basic_istream<Ch,Tr>& std::operator>>(std::basic_istream<Ch,Tr>& in,
                                           Canvas& o)
{
    // ...
}

This will give you the extra flexibility of supporting any stream at the cost of the same increased compilation time you have now.

wilhelmtell
A: 

There is also an error in the way You initialize constant members of Your class.

You have to use an initialization list

Canvas::Canvas()
: Nrow(0), Ncol(0)
{
  Title = "";
  //Nrow = 0; - this is an attempt to change the value of a const, which was already constructed.
  //Ncol = 0; - same as above
  //image[][100] = {};
  position.r = 0;
  position.c = 0;
}
Maciej Hehl