newbie: object out of scope?

[deko]

I'm having a problem trying to run methods in the DataAccess layer of my
WinForms app. I'm wondering if the problem is a result of the way I've
designed the interaction between the different layers.

I have my app organized in 3 projects and (for the sake keeping this simple)
4 classes:

App_DA
DataAccess.cs
Helper.cs
App_BL
BusinessLogic.cs
App_UI
FormMain.cs

In the constructor of the main form, I instantiate a Helper object:

public FormMain()
{
InitializeComponent();
Helper helper = new Helper();
}

which in turn instantiates a DataAccess object:

public Helper()
{
DataAccess da = new DataAccess();
}

The problem happens when I call a method in the DataAccess class from
FormMain:

private void button1_Click(object sender, EventArgs e)
{
DataAccess da = new DataAccess();
da.updateDataSet
}

The updateDataSet method is not updating the DataSet.

Could this be caused by the way I'm instantiating the objects in the
different layers?

Is it a bad idea to instantiate other objects in constructors like I am
doing here?

Thanks in advance.

 

[Willy Denoyette [MVP]]

| I'm having a problem trying to run methods in the DataAccess layer of my
| WinForms app. I'm wondering if the problem is a result of the way I've
| designed the interaction between the different layers.
|
| I have my app organized in 3 projects and (for the sake keeping this
simple)
| 4 classes:
|
| App_DA
| DataAccess.cs
| Helper.cs
| App_BL
| BusinessLogic.cs
| App_UI
| FormMain.cs
|
| In the constructor of the main form, I instantiate a Helper object:
|
| public FormMain()
| {
| InitializeComponent();
| Helper helper = new Helper();
| }
|
| which in turn instantiates a DataAccess object:
|
| public Helper()
| {
| DataAccess da = new DataAccess();
| }
|

What's the use of this? Your da has local scope, that means that once the
constructor has done it goes out of scope and it's contents (a reference to
the DataAccess object) gets nulled.

| The problem happens when I call a method in the DataAccess class from
| FormMain:
|
| private void button1_Click(object sender, EventArgs e)
| {
| DataAccess da = new DataAccess();
| da.updateDataSet
| }
|

This should work, here da is local, but you call updateDataSet using this
local reference. Once the method returns the variable goes out-of-scope and
the object instance it referred to becomes eligible for GC. Probably there
is something wrong with the method itself, running this in the debugger
might help you out.



| The updateDataSet method is not updating the DataSet.
|
| Could this be caused by the way I'm instantiating the objects in the
| different layers?
|
| Is it a bad idea to instantiate other objects in constructors like I am
| doing here?
|

See above.

Willy.

 

[Marc Gravell]

Your code doesn't make it entirely clear exactly what you are doing, since
nothing involving the DataSet (i.e. where it lives) is shown; hence, I am
piecing things together here, and could be wrong:

Is it a bad idea to instantiate other objects in constructors like I am
doing here?

Well, normally : no - in fact it is very common - however in this case it
doesn't do much:

public Helper()
{
DataAccess da = new DataAccess();
}
public FormMain()
{
InitializeComponent();
Helper helper = new Helper();
}

Once either of the above has completed, your new Helper / DataAccess object
is sat on its own looking lonely - nothing can see it, and the GC is going
to be lurking around the corner with a black robe and a scythe.

The updateDataSet method is not updating the DataSet.

I think it probably is, but it is updating a *different* DataSet instance,
which is the GCd; when you call:

private void button1_Click(object sender, EventArgs e)
{
DataAccess da = new DataAccess();
da.updateDataSet
}

this creates an *entirely different* instance of DataAccess - and merrily
does whatever your code says; if you then do something like the following to
try and inspect the object, you will see a third instance:

private void CheckMe() {
DataAccess da = new DataAccess();
da.ShowCurrentState(); // invented
}

So: various solutions; one is to place your DataAccess object onto the
Helper so that every use of the Helper uses the same instance of the
DataAccess object:

public class Helper {
private DataAccess da;
public Helper()
{
da = new DataAccess();
}
public DataAccess DataAccess {get {return da;}} // so callers can access
da

public void SomeHelperMethod() {
// do something with the shared copy of da - e.g. updateDataSet
}
}

likewise:

public class FormMain {
private Helper helper
public FormMain()
{
InitializeComponent();
Helper helper = new Helper();
}
}
You could then point button1_Click to go through the Helper, either using
helper.DataAccess, or using helper.SomeHelperMethod()

You can also achieve a lot of this using static methods etc - however, this
limits you (effectively) to a single instance, and can be restrictive e.g.
if you want different Form instances to own different DataAccess instances
(via separate Helper instances)

Does that help?

Marc

 

[Marc Gravell]

Clipboard error : final code block should read:

public class FormMain {
private Helper helper
public FormMain()
{
InitializeComponent();
helper = new Helper(); // note change; updating existing "helper"
field
}
}

 

[deko]

Thanks for the detailed reply.

Your code doesn't make it entirely clear exactly what you are doing, since
nothing involving the DataSet (i.e. where it lives) is shown; hence, I am
piecing things together here, and could be wrong:

I've fleshed things out below.

Well, normally : no - in fact it is very common - however in this case it
doesn't do much:

Once either of the above has completed, your new Helper / DataAccess
object is sat on its own looking lonely - nothing can see it, and the GC
is going to be lurking around the corner with a black robe and a scythe.

great explanation

I think it probably is, but it is updating a *different* DataSet instance,
which is the GCd; when you call:

private void button1_Click(object sender, EventArgs e)
{
DataAccess da = new DataAccess();
da.updateDataSet
}

this creates an *entirely different* instance of DataAccess - and merrily
does whatever your code says; if you then do something like the following
to try and inspect the object, you will see a third instance:

I see.
As an aside, is there an easy way to count the number of instances of the
DataAccess object? That might be a good troubleshooting step.

So: various solutions; one is to place your DataAccess object onto the
Helper so that every use of the Helper uses the same instance of the
DataAccess object:

public class Helper {
private DataAccess da;
public Helper()
{
da = new DataAccess();
}
10-4

public DataAccess DataAccess {get {return da;}} // so callers can access
da

What is the above code doing? Is it creating a read-only propery in the
DataAccess object?

You can also achieve a lot of this using static methods etc - however,
this limits you (effectively) to a single instance, and can be restrictive
e.g. if you want different Form instances to own different DataAccess
instances (via separate Helper instances)

Does that help?

Helps a lot. So the problem is, essentially, the object's scope (or my
misunderstanding of the object's scope).

Here's a more detailed description of the issue:

In an effort to keep all data access in the DA layer, I create a DataSet
(from xml) in the DA layer (DataAccess.cs). But the DataSet needs to be
updated by different forms in the UI layer (and needs to be accessed by
objects in the BL layer as well). So the core issue is how to do this.

I've read up a bit and discovered the GetChanges method of the DataSet
object. Apparently, the benefit is limiting the amount of data passed
between layers. But I still have to MAKE the changes before I can GET the
changes - so I would have to fill a DataSet twice: once in the DA layer, and
again in the UI layer (contrary to the idea of keeping data access in the DA
layer). THEN, I could pass only the changes into the DA layer with a method
(in the da object) something like this:

DataSet form = formInstanceDataSet
DataSet thirdDataSet = form.GetChanges();

While that may limit the amount of data passed between layers, it seems less
efficient than your suggestion of making the DataSet a read-only property of
the DataAccess object. Furthermore, how would I persist only the changes?
Currently, I serialize the DataSet like this:

XmlSerializer ser = new XmlSerializer(typeof(DataSet));
TextWriter writer = new StreamWriter(xmlFile);
ser.Serialize(writer, myDataSet);

Why should I abandon the simplicity and atomicity of this to update only the
changes?

It sounds like the way to go is with a read-only public property of the
DataAccess object, as you suggested. That property can be available to
whatever form (object) needs it. Then on close of the form I can call a
method in the DataAccess class to serialize the DataSet back to xml.

Does this sound about right?

 

[deko]

| I have my app organized in 3 projects and (for the sake keeping this

simple)
| 4 classes:
|
| App_DA
| DataAccess.cs
| Helper.cs
| App_BL
| BusinessLogic.cs
| App_UI
| FormMain.cs
|
| In the constructor of the main form, I instantiate a Helper object:
|
| public FormMain()
| {
| InitializeComponent();
| Helper helper = new Helper();
| }
|
| which in turn instantiates a DataAccess object:
|
| public Helper()
| {
| DataAccess da = new DataAccess();
| }
|

What's the use of this? Your da has local scope, that means that once the
constructor has done it goes out of scope and it's contents (a reference
to
the DataAccess object) gets nulled.

I think that's my problem.

| The problem happens when I call a method in the DataAccess class from
| FormMain:
|
| private void button1_Click(object sender, EventArgs e)
| {
| DataAccess da = new DataAccess();
| da.updateDataSet
| }
|

This should work, here da is local, but you call updateDataSet using this
local reference. Once the method returns the variable goes out-of-scope
and
the object instance it referred to becomes eligible for GC. Probably there
is something wrong with the method itself, running this in the debugger
might help you out.

I also found this:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnbda/html/BOAGag.asp

68 pages.... but it should provide some foundational understanding.

 

[Marc Gravell]

As an aside, is there an easy way to count the number of instances of the

DataAccess object?

Well, you can count how many have been *created*; it is harder to count how
many are
in existance (i.e. haven't been destroyed), as this depends on when it is
GC'd; you could
use a finalizer, but IMO the *only* use of finalizers should be in wrappers
around
unmanaged resources, so I can't bring myself to recommend this route...

Anyway, to count how many have been created, use a static field; in this
code I have
also provided thread-safety around the counter (in case two threads are
creating at the same time), but don't
panic about all the "lock" stuff - the main point here is the counter:

public class SomeClass {
private static int countCreated = 0;
private static object syncLock = new object();

public static int CountCreated {
get {
lock (syncLock) {
return countCreated;
}
}
}
private static void IncrementCounter() {
lock (syncLock) {
countCreated++;
}
}
public SomeClass() {
IncrementCounter();
}
}
public class Program {

static void Main(string[] args) {
new SomeClass();
new SomeClass();
new SomeClass();
Console.WriteLine(SomeClass.CountCreated);
}
}

What is the above code doing? Is it creating a read-only propery in the
DataAccess object?

Yes; it depends on how you intend to use it, but to me it seemed reasonable
that the form
might need direct access to the DataAccess instance; when accessed in this
way you
can't accidentally swap instances. If you knew that you would *never* want
to change
the DataAccess instance held in the Helper object, I would even recommend
declaring
it as readonly in the Helper class - this prevents you introducing bugs here
(accidentally); just
add the keyword "readonly":

private readonly DataAccess da;

Apparently, the benefit is limiting the amount of data passed between
layers.

Actually, no. As long as you are working in one app-domain, passing an
entire DataSet between layers (UI / DB etc)
involves no more work than passing an object containing the changes; either
way, all it takes is a handful
of bytes to pass the address of the object. The *real* benefit is when
passing an object *outside*
of your app-domain - i.e. to pass just the changes to a web-service, or a
file-system, or a different app-domain.
In any of these, the object must first be serailized - and it is better to
transfer only the data we actually
care about.

Why should I abandon the simplicity and atomicity of this to update only
the

changes?

It depends on what you are doing with the serialization! If this is a file
on the client HDD, then I wouldn't
change a thing: it makes perfect sense to keep the entire DataSet together
rather than having lots of
delta files. If you are passing it to a web-service (or similar) then the
reason would be size: only send what
you actually want to work with at the other end.
Also - you might want to look at the WriteXml method - it might be simpler
to use in this scenario.

While that may limit the amount of data passed between layers, it seems
less efficient than your suggestion of making the DataSet a read-only
property of the DataAccess object.

Actually, I didn't suggest that; I spoke about the DataAccess object
(because your code didn't
really mention the DataSet). However, this approach (DataSet as a property)
could also be a good idea.

Hope this helps,

Marc

 

[deko]

but IMO the *only* use of finalizers should be in wrappers around

unmanaged resources, so I can't bring myself to recommend this route...
10-4

Anyway, to count how many have been created, use a static field; in this
code I have
also provided thread-safety around the counter (in case two threads are
creating at the same time), but don't
panic about all the "lock" stuff - the main point here is the counter:

I see. So just put a counter in the class...

Actually, no. As long as you are working in one app-domain, passing an
entire DataSet between layers (UI / DB etc)
involves no more work than passing an object containing the changes;
either way, all it takes is a handful
of bytes to pass the address of the object. The *real* benefit is when
passing an object *outside*
of your app-domain - i.e. to pass just the changes to a web-service, or a
file-system, or a different app-domain.
In any of these, the object must first be serailized - and it is better to
transfer only the data we actually
care about.
10-4

changes?

It depends on what you are doing with the serialization! If this is a file
on the client HDD, then I wouldn't
change a thing: it makes perfect sense to keep the entire DataSet together
rather than having lots of
delta files. If you are passing it to a web-service (or similar) then the
reason would be size: only send what
you actually want to work with at the other end.
Also - you might want to look at the WriteXml method - it might be simpler
to use in this scenario.

That makes sense - the world of web services has different needs.

I've decided to go with a Singleton class for this issue, but not sure how
to incorporate my methods into that pattern. So I've reposted that question
[newbie: How to construct Singleton pattern?].

Thanks for your help!