I'm going to just vent here folks... In programming, if you have a class which HAS TO HAVE all its members initialised, DO NOT just expose an empty constructor...
Here's what I mean...
#include <string>
using namespace std;
class A
{
public:
int _id;
string _name;
A (const int& id, const string& name)
:
_id(id),
_name(name)
{
}
};
There's a class right, one assumes there's only one way to initialise a copy of this class? The constructor giving implicite values, yes?... Good.
So when you want this pattern, make sure it works just how you expect, hide the other constructors... thus:
#include <string>
using namespace std;
class A
{
private:
A() {}
public:
int _id;
string _name;
A (const int& id, const string& name)
:
_id(id),
_name(name)
{
}
};
This stops blank copies being generated, on compilers which automatically create the defautl constructor public!
Likewise in C#, do the same... If you have a data class:
class B
{
public int _id { get; set; }
public string _name { get; set; }
public B ()
{
}
public B (int id, string name)
{
_id = id;
_name = name;
}
public string NewToString ()
{
return string.Format("{0}-{1}", _id, _name);
}
}
That class is no big deal, the int and string are implicit values, they have inplicit values upon creation.
B b = new B();
Console.writeline (b.NewToString());
All works fine, but if we add a user class in there, or any other class which can be null... then we have to be more careful...
class StudentRecord
{
public string _name { get; set; }
public DateTime _dob { get; set; }
}
class B
{
public int _id { get; set; }
private StudentRecord _student;
public string _name
{
get { _student._name; }
set { _student._name = value; }
}
public B ()
{
}
public B (int id, string name)
{
_id = id;
_student = new StudentRecord();
_name = name;
}
public string NewToString ()
{
return string.Format("{0}-{1}", _id, _name);
}
}
What happens now?.... Well the code will crash:
B b = new B();
Console.writeline (b.NewToString());
Will throw a null reference exception in the call _name against _student, as it is not implicitely defined during creation when using the default constructor.
What we should really do is utilise the "Resource Allocation Is Initialisation" (RAII) concept and only allow a new B to be created with the defined values. And we should do this to all classes, whether they have implicit values or user defined classes within.
How do we do this? By stopping the default constructor being used:
class B
{
public int _id { get; set; }
private StudentRecord _student;
public string _name
{
get { _student._name; }
set { _student._name = value; }
}
private B ()
{
}
public B (int id, string name)
{
_id = id;
_student = new StudentRecord();
_name = name;
}
public string NewToString ()
{
return string.Format("{0}-{1}", _id, _name);
}
}
Now, the only way for the programmer to get using this wrong is to mess about with the code!
It stops all those annoying "What the Hell just happened" during runtime, and some poor other schmuck doesn't have to do running around working out where your code went wrong because it missed an assignment, such as this, very poor code:
class B
{
public int _id { get; set; }
public StudentRecord _student { get; set; }
public string _name
{
get { _student._name; }
set { _student._name = value; }
}
private B ()
{
}
public string NewToString ()
{
return string.Format("{0}-{1}", _id, _name);
}
}
B b = new B();
b._id = 0;
b._name = "Tom";
Which would need to be:
B b = new B();
b._id = 0;
b._student = new StudentRecord();
b._name = "Tom";
Horrid horrid, its bad code, bad coding practice, and using excuses like "This class will be removed later" doesn't really wash with me, because code has a half life you can never predict, you don't know how long your code, or your class, will be around and used and release it in a bad form today, it'll bite you again and again... Release it nicely, with rigourous defensive programming and you'll find yourself not being hassled by it.
No comments:
Post a Comment