Re: matrices
From: Victor Bazarov (v.Abazarov_at_comAcast.net)
Date: 12/03/04
- Next message: Dfschweiss: "Re: A Question about std::list"
- Previous message: Howard: "Re: Removing const for benefit of assignment operator"
- In reply to: tommy: "matrices"
- Next in thread: Rade: "Re: matrices"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Fri, 03 Dec 2004 10:29:23 -0500
tommy wrote:
> I'm trying to write a program which will be adding and subtracting
> two matrices. I've already written some code but there are a few problems.
>
> 1) During compilation I get this warning:
> mt.cpp 17: temporary used for parameter 1 in call to: istream: operator >>
> (int &)
Really? The only 'cin' I see is in the 'fill' function and it doesn't
look like a use of a temporary to me... Hard to say. Your code is non-
standard and I don't have BC++ v3.1 to verify your claim.
> 2) When I'm trying to show results of addition and subtraction two matrices
> I get some strange really big unexpected numbers.
>
> The source code is bellow. I use Borland C++ v3.1
That's about as old a compiler as one can find still in use. Perhaps you
should consider at least upgrading to the [free] v5.5...
Now, about the code...
> ----------
> def.h
> ----------
> #ifndef _DEF_H_
> #define _DEF_H_
Names with leading underscore followed by a capital letter are reserved by
the implementation. There is no need for you to have that fancy leading
underscore here. Be simple.
>
> #include <iostream.h>
There is no standard header <iostream.h>. Use <iostream> and make sure to
change your code appropriately.
> #include <conio.h>
There is no standard header <conio.h>. Whatever may arise from using it
is not guaranteed by the language.
> #include "mt.h"
>
> #endif
> ---------
> mt.h
> ---------
> #ifndef _MT_H_
> #define _MT_H_
>
> class Mt { /* multidimensional table */
Misleading comment. It's really has a _two_ dimensional "table" and of
very specific size.
> public:
> Mt() ;
> void fill() ;
> friend void operator+(Mt &,Mt &) ;
> friend void operator-(Mt &,Mt &) ;
Seems like two operators. Good. But why do they return 'void'? Do you
not want some result from those operations recorded?
Change those to
friend Mt operator+(Mt const&, Mt const&);
friend Mt operator-(Mt const&, Mt const&);
And add
void print() const; // think of how you'd implement it.
> private:
> int values[5][5] ;
> const int firstcolumn ; /* number of first column in table
> */
> const int firstline ; /* number of first line in table */
> int column ;
> int line ;
These two members above don't have any comment next to them. What are
they for?
> const int nc ; /* number of columns */
> const int nl ; /* number of lines */
> } ;
>
> #endif
> --------
> main.cpp
> --------
> #include "def.h"
This is a rather bad practice if you ask me. It is much better to
explicitly include all headers than do it indirectly through another
header. Doing it indirectly does NOT help improve compilation speed
and only adds confusion when one is looking at the 'main.cpp' module.
>
> int main(int , char *[]) /* argument counter, argument vector */
> {
> clrscr() ;
Non-standard function. Try to avoid those until your program works.
>
> Mt m1, m2 ;
> m1.fill() ;
> m2.fill() ;
>
> m1 + m2 ;
> m1 - m2 ;
These two statements above are very strange. Imagine I'd write
int i1, i2;
i1 = 1;
i2 = 2;
i1 + i2;
What does the addition do? Nothing. It adds two values and the sum is
simply thrown away. That's what the m1+m2; looks to me.
Read a good book about operator overloading. Find a copy of "Effective
C++" by Scott Meyers, you seem to be in desperate need of reading it.
The code in 'main' should be changed to
Mt msum = m1 + m2;
Mt mdiff = m1 - m2;
.. // somehow output the contents of 'msum' and 'mdiff'
>
> getch() ;
> return 0 ;
> }
> -------
> and mt.cpp
> -------
> #include "def.h"
Again, avoid using those "collection" headers. Be explicit.
>
> Mt::Mt() : firstcolumn(0), firstline(0), nc(5), nl(5)
Kudos! That's a very rare thing to see a novice who uses initialiser
lists instead of assignments. Good job!
> {
> ; /* do nothing */
You don't need a semicolon here to do nothing. You may leave the comment
in if you want but many would understand that
{
}
does nothing.
> }
>
> void Mt::fill()
> {
> column = firstcolumn ;
>
> while( column < nc )
> {
> for(line = firstline ; line < nl ; line++)
Rather strange choice of loop controlling statements. Why not
both 'while' or both 'for'?
> {
> cout << "i= " << column << " j= " ;
Prints
i= <number> j=
Did you forget to output 'line'?
> cin >> values[column][line] ;
> }
>
> column++ ;
> }
> }
>
> void operator+(Mt &first, Mt &second)
Well, if you read the "Effective C++", you'll know how you should declare
the addition operator:
Mt operator +(Mt const &first, Mt const &second)
> {
> int sum[5][5] ;
This should probably be
Mt sum;
>
> // Let's make some addition of these two tables
>
> int column = 0 ;
> int line = 0 ;
>
> cout << "\nAddition: \n" ;
You might want to leave this in for debugging purposes, but really your
addition operator shouldn't be telling everybody it's doing its job.
>
> while( column < 5 )
> {
> for(line = 0 ; line < 5 ; line++)
> sum[column][line] = first.values[column][line] +
> second.values[column][line] ;
Since I recommend changing 'sum' to be of type Mt, this should become
sum.values[column][line] = ...
>
> column++ ;
> }
>
> column = 0 ;
>
> while( column < 5 )
> {
> if ( column <= 5 - 1)
> cout << "\n" ;
>
> for(line = 0 ; line < 5 ; line++)
> cout << sum[line][column] << " " ;
>
> column++ ;
> }
> cout << "\n\n" ;
You should consider creating a special _separate_ function for outputting
the result (an Mt object)
And, of course, since we changed the return value type, you need
return sum;
here.
> }
>
> void operator-(Mt &first, Mt &second)
> [...]
Same notes as with the operator +.
All in all, a very good effort! Now, get a copy of "Effective C++" and
consume it.
Victor
- Next message: Dfschweiss: "Re: A Question about std::list"
- Previous message: Howard: "Re: Removing const for benefit of assignment operator"
- In reply to: tommy: "matrices"
- Next in thread: Rade: "Re: matrices"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
