Welcome!!
I'm adding my source code in here to recieve criticism from other programmers.
Welcome!!
I'm adding my source code in here to recieve criticism from other programmers.
Im sorry to say this but its not too good at all.
first u mix include headers using some using std namespace and some not.This isnt a good start especially as none of those headers are used in main.h.Then using namespace std brings the whole of the namespace into scope when in reality you are not using any of it.
You should prefer using initialisation list instead of constructor body to initialise members.Only use body if u have to.No checking either for bounds. Not sure how class is to be used. Will negative values here make sense? Should u check X and Y before initialising members.
Just for aesthitic value a virtual destructor wouldnt have gone amiss. That would make this class more value as a base class.
operator + is ok as u have done it but again would this make sense still if x and y are negative?
friend int main() WTF what,why,are you trying to do? Why on earth should main be a friend of this class.Thats preposterous.
Information need not be a friend either. It uses accessor functions to access the classes internal data and does not need direct access.
Magnitude needs rewriting and also does not need to be a friend for same reasons as Information.
Your accessor functions are laughable. Where is your const correctness?!
Another word of advice.... In c++ we have references. They can dramatically reduce the need for passing pointers into functions.
I'd seriously consider a rewrite.
Free the weed!! Class B to class C is not good enough!!
And the FAQ is here :- http://faq.cprogramming.com/cgi-bin/smartfaq.cgi
Here is a better way. You have to be careful
with the normalization since you can have 0 vectors.
Right now I comparing floating point numbers to 0.0 but to
make it more robust you should compare it to a range of
numbers near 0. You can improve this further by maybe using
templates, adding substracting, multiplication etc. Another
way to handle the normalization is to return another vector
and make the action function const.
Code:#ifndef VECTOR_H__ #define VECTOR_H__ #include <stdexcept> #include <cmath> class Vector { double x; double y; public: Vector(double new_x = 0.0, double new_y = 0.0) : x(new_x), y(new_y) { } Vector& operator+=(const Vector& rhs) { x += rhs.x; y += rhs.y; return *this; } Vector operator+(const Vector& rhs) { Vector& v = *this; v += rhs; return v; } double get_x() const { return x; } double get_y() const { return y; } double magnitude() const { return std::sqrt(x * x + y * y); } void normalize() { double m = magnitude(); if (m == 0.0) throw std::runtime_error("Divide by 0"); x /= m; y /= m; } }; #endif #include <iostream> using std::cout; using std::endl; #include "Vector.h" int main(void) { Vector v(1, 9); Vector u(4, 5); Vector w; cout << "v = (" << v.get_x() << ", " << v.get_y() << ")" << endl; cout << "u = (" << u.get_x() << ", " << u.get_y() << ")" << endl; w = u + v; cout << "w = u + v" << endl; cout << "w = (" << w.get_x() << ", " << w.get_y() << ")" << endl; cout << "v's magnitude = " << v.magnitude() << endl; cout << "u's magnitude = " << u.magnitude() << endl; cout << "normalizing" << endl; v.normalize(); u.normalize(); cout << "v = (" << v.get_x() << ", " << v.get_y() << ")" << endl; cout << "u = (" << u.get_x() << ", " << u.get_y() << ")" << endl; cout << "v's magnitude = " << v.magnitude() << endl; cout << "u's magnitude = " << u.magnitude() << endl; return 0; }
How would that be done? I thought that delete could only be used if new was used. What would you put into the implementation of the destructor, then?Originally posted by Stoned_Coder
Just for aesthitic value a virtual destructor wouldnt have gone amiss. That would make this class more value as a base class.
FAQ
"The computer programmer is a creator of universes for which he alone is responsible. Universes of virtually unlimited complexity can be created in the form of computer programs." -- Joseph Weizenbaum.
"If you cannot grok the overall structure of a program while taking a shower, you are not ready to code it." -- Richard Pattis.
declare it in the class as virtual ~classname(); and in the implementation file add a do nothing implementation.
class MyClass
{
virtual ~MyClass();
};
MyClass::~MyClass() {}
If you have no intention of ever using this as a base class then this is probably overkill,if on the other hand you intend to derive from this class and use polymorphically then a virtual destructor is necessary.
Free the weed!! Class B to class C is not good enough!!
And the FAQ is here :- http://faq.cprogramming.com/cgi-bin/smartfaq.cgi
Heres the revision
"A Programmer being told to 'go to' hell sees the 'go to' part of the sentence as the worst part." - Master Foto
ok here we go again.
accessor functions should be const.
PrimTypebyValue GetPrivateMember()const { return PrivateMember;}
or
const UserDefTypebyConstRef& GetPrivateMember()const { return PrivateMember;}
operator += should return *this
iostream is not used in main.h it should be included in main.cpp instead
main does never need to be prototyped.
Information() should take a reference not a pointer.
(*Vector).getX() is better written Vector->getX()
Free the weed!! Class B to class C is not good enough!!
And the FAQ is here :- http://faq.cprogramming.com/cgi-bin/smartfaq.cgi
Nick your operator + is wrong. It changes *this....
a=b+c you dont expect b to be changed. *this should be const throughout operator +
Free the weed!! Class B to class C is not good enough!!
And the FAQ is here :- http://faq.cprogramming.com/cgi-bin/smartfaq.cgi
Why should accessor functions be const?
const objects are only allowed to call upon const member functions and cannot call non const member funcs.
i.e.
const CVector vec(10,10); // a const object
int a = vec.getX(); // ERROR unless function is const member and it should be because it doesnt change the state of the object.
Free the weed!! Class B to class C is not good enough!!
And the FAQ is here :- http://faq.cprogramming.com/cgi-bin/smartfaq.cgi