General Design Question (should be easy for you)

[zackiv31]

I have a program that I'm trying to break into multiple files for proper coding and easy expansion in the future. I have a button that scans the computer for certain attributes, and as it scans, I want it to update a ListBox (lstBox)... I get errors because I don't think I'm laying it out correctly... My code is below:

namespace abc
{
	public interface IObserver
	{
		void update();
	}
}

namespace abc
{
	public partial class Form1 : Form
	{
		....

		class lstViewObserver : IObserver
		{
			public void update()
			{
					  .... //Here is where it cannot access lstView.
			}
		 }
	}

That's how my code's setup now, and I get this error:

Error	5	Cannot access a non-static member of outer type 'abc.Form1' via nested type 'abc.Form1.lstViewObserver'

If i put the interface in the Form1, it responds with a different error, because of another file I have.

Scan.cs

namespace abc
{
	public class Scan
	{
	   ...
		 public void RegisterObserver(IObserver observer)
		{
		  ...
		 }
	   }
  }

Error	5	Cannot access a non-static member of outer type 'zives.Form1' via nested type 'zives.Form1.lstViewObserver'	C:\Users\zivester\Documents\Visual Studio 2008\Projects\zives\zives\Form1.cs	259	17	zives

 

[MadJock]

Hi zackiv31,

The list view is private to the Form1 class.

If you want to use a nested class, one approach is listed below:

namespace abc
{
    public partial class Form1 : Form
    {
        ....

        class lstViewObserver : IObserver
        {
            private Form1 _parent;
            public lstViewController(parent Form1){
              _parent = parent;
            }
            public void update()
            {
                      _parent.ListView1.DoSomething()            }
         }
    }

Hope this helps!

"Just beacuse you're paranoid, don't mean they're not after you

 

[MadJock]

Sorry - don't pass a Form1 to lstViewObserver - pass a ListView!



"Just beacuse you're paranoid, don't mean they're not after you

 

[zackiv31]

So I shouldn't be passing in the parent? I have to update a couple things besides the listView (a couple text boxes as well... should I just pass in the whole parent, or can it be accessed explicitly?)

I'm so close.. thanks for the help so far.

 

[JurkMonkey]

I can't believe I'm reading this.

You need to re-read the observer pattern. No form should ever know about another form. That is by definition, bad design.

You want a controller class that takes care of access to the information that you display - whether it be in a list, a dropdown, a table, a textbox, anything. When one form updates the data via the controller, the controller tells all the other forms to update itself using the interface you have provided. The beauty of an interface is the fact that your controller doesn't care if its a form, a control, a website, anything. Your controller just calls the method that is guaranteed to be on that view.

This is known as the Model-View-Controller (MVC) pattern.

Do some googling - it will help you with future expansions.

http://www.ubuntu.com/

 

[zackiv31]

I know about MVC and I've done some with php... the fact is I taught myself C# just from googling about it. I learned java 5 years ago and haven't really touched it since. My expertise is in systems level programming, so I'm unfamiliar with proper design goals, and thus why I am asking about it. You're response doesn't really help me that much, unless you could show me an example of a C# application with MVC pattern. I will google again now to look it up. I'm just trying to convert a program I wrote for speed (a timeframe) to one that is easily expandable, never done something like this.

Sorry you can't believe it.

 

[JurkMonkey]

zackiv31,

I was not not in disbelief about your question but about the advice you were being given.

Try some basic solutions using MVC and the Observer pattern. This will make your forms disconnected from eachother.

http://www.ubuntu.com/

 

[chrissie1]

C# is not java in .net you can easily make the observer pattern workk with events.

the controller would for example have an event datachanged and the views (forms, lists, combos) would consume that.

So if your controller has any reference to system.windows.forms then it is wrong. If your controller has any reference to system.data in any form it is wrong.

Christiaan Baes
Belgium

My Blog
"In a system where you can define a factor as part of a third factor, you need another layer to check the main layer in case the second layer is not the base unit." - jrbarnett

 

[zackiv31]

Sorry to dig this one up but I cannot for the life of me get this to work :-/ I've tried using delegates and such from an MVC idea..

http://www.codeproject.com/csharp/model_view_controller.asp

Here is a summary of my current problem:

I want to run a backgroundWorker that scans my computer for things. Every time it completes a part of the scan, I want it to update the ListView with what operation just completed (and anything else attached to it). I've tried this using delegates, but when I try to update the View, I get a cross-threaded exception.. because I'm running it in a background worker i assume? If I don't use a background worker, its not async, so the gui freezes until scan is done.

Here's a slice of my code now:

        private void btnScan_Click(object sender, EventArgs e)
        {
            prgsTemp.Style = ProgressBarStyle.Marquee;
            prgsTemp.Update();
            txtInfo.Text = "Scan In Progress...";
            txtInfo.Update();
            
            /*BackgroundWorker worker = new BackgroundWorker();
            worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
            worker.DoWork += new DoWorkEventHandler(worker_DoWork);
            worker.RunWorkerAsync();*/
            worker_DoWork(null, null);
        }

        private Scan.ScanChangedHandler scanDelegate;
        private Scan s;

        private void worker_DoWork(object sender, EventArgs e)
        {
            s = new Scan();
            scanDelegate = new Scan.ScanChangedHandler(scanChanged);
            s.ScanChanged += scanDelegate;
            s.scan();
        }

        public void scanChanged(object sender, ScanChangedEventArgs e)
        {
            ListViewItem lvi = new ListViewItem(s.directory);
            lvi.Text = s.directory;
            txtTotalSize.Text = Files.fileSize(s.totalSize);
            txtTotalSize.Update();
            txtTotalFiles.Text = s.totalFiles.ToString();
            txtTotalFiles.Update();
            lvi.SubItems.Add(s.currentSizeInfo.getNumber().ToString());
            lvi.SubItems.Add(Files.fileSize(s.currentSizeInfo.getSize()));
            lstView.Items.Add(lvi);
            lstView.Update();
        }

 

[JurkMonkey]

private void worker_DoWork(object sender, EventArgs e)
{
s = new Scan();
scanDelegate = new Scan.ScanChangedHandler(scanChanged);
s.ScanChanged += scanDelegate;
s.scan();
}


This is where your issue lies. You are creating a thread and then attaching to the delegate on the new thread. You actually want to attache to the delegate on the main thread and then your new thread just calls it.


There are 2 ways you can get around this - the right way and the hack way. I'll start with the right way.

 private void btnScan_Click(object sender, EventArgs e)
        {
            prgsTemp.Style = ProgressBarStyle.Marquee;
            prgsTemp.Update();
            txtInfo.Text = "Scan In Progress...";
            txtInfo.Update();

//Create your new scan and attach to the events here
s = new Scan();
            scanDelegate = new Scan.ScanChangedHandler(scanChanged);
            s.ScanChanged += scanDelegate;

            
            /*BackgroundWorker worker = new BackgroundWorker();
            worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
            worker.DoWork += new DoWorkEventHandler(worker_DoWork);
            worker.RunWorkerAsync();*/
            worker_DoWork(null, null);
        }

        private Scan.ScanChangedHandler scanDelegate;
        private Scan s;

        private void worker_DoWork(object sender, EventArgs e)
        {
            s.scan();
        }

        public void scanChanged(object sender, ScanChangedEventArgs e)
        {
            ListViewItem lvi = new ListViewItem(s.directory);
            lvi.Text = s.directory;
            txtTotalSize.Text = Files.fileSize(s.totalSize);
            txtTotalSize.Update();
            txtTotalFiles.Text = s.totalFiles.ToString();
            txtTotalFiles.Update();
            lvi.SubItems.Add(s.currentSizeInfo.getNumber().ToString());
            lvi.SubItems.Add(Files.fileSize(s.currentSizeInfo.getSize()));
            lstView.Items.Add(lvi);
            lstView.Update();
        }

The hack solution would be to check if your event is being called on the GUI thread and then invoke it if it isn't.

public void scanChanged(object sender, ScanChangedEventArgs e)
{
if (txtTotalSize.InvokeRequired)
{
this.Invoke(new ScanChangedHandler(), new object[] {sender, e});
}
else
{
ListViewItem lvi = new ListViewItem(s.directory);
lvi.Text = s.directory;
txtTotalSize.Text = Files.fileSize(s.totalSize);
txtTotalSize.Update();
txtTotalFiles.Text = s.totalFiles.ToString();
txtTotalFiles.Update();
lvi.SubItems.Add(s.currentSizeInfo.getNumber().ToString());
lvi.SubItems.Add(Files.fileSize(s.currentSizeInfo.getSize()));
lstView.Items.Add(lvi);
lstView.Update();
}
}


This method will see if it is on the correct thread and then re-invoke itself if it is not. The second time in it should be on the gui thread and will go to the else statement.

http://www.ubuntu.com/

 

[zackiv31]

JurkMonkey,

Are you sure the "right way" is correct? It works but it freezes the gui until the scan is complete... The list updates ok (because backgroundWorker is still commented out I think?).

I need it to scan in the background and update the same way it does right now.. how can I change it to do this?

 

[JurkMonkey]

How often are you updating the GUI thread? Everytime you update the GUI thread the GUI will freeze until the method is complete. So if you're updating the GUI 5 times a second then yes, chances are it will appear to be frozen.

http://www.ubuntu.com/

 

[zackiv31]

Ok I'd agree with you.. but my program hits one directory in it's scan that doesn't call an update for about 10 seconds.. and the gui is still frozen there.

So I don't think it's asynchronous then right?

 

[JurkMonkey]

pttttttht! Look at your code...

worker.RunWorkerAsync();

That is the line that starts the worker in a separate thread and it is commented out. Put it back in and comment out worker_DoWork(null, null);

http://www.ubuntu.com/

 

[zackiv31]

That doesn't work... I had the comments around it because I already tried that

It gives cross threading exception.. because the background worker tries to access elements of the form (like txtTotalSize).

If this is the way I should be going, how do I get around this hurdle?

 

[JurkMonkey]

Ok.... This is going to be a simple lesson in threading and design.

GUI
|
| Read Folder Thread
|-----|
| |
| |
|<---/ Notify the main thread of progress
| |
| |
|<---- Thread Completes
|
|


At no point should a second thread attempt to update a GUI item. It should only notify another object that something is done via a delegate. (Invoke)

The only thing that should know about a textbox on a form is the form that contains it. Something that reads folders should not be in your form and it should not even know about your form. This way it can do work in a separate thread.

http://www.ubuntu.com/

 

[zackiv31]

It's a little late so I'm kind of tired... but I don't see how my code doesn't follow that design... all the above code is in the Form1 (main form). The Scan class is in it's own file and doesn't do anything with actualy Form data, as you said it shouldn't. Whenever the Scan hits a point where it should update, it should call scanChanged() in the main Form1.

Or maybe it doesn't call it.. but the event should be fired, but its within the same Scan class, so that's why I'm getting the error?

public class Scan
    {
... .. .. ..

        // declare a delegate for the scanchanged event
        public delegate void ScanChangedHandler(object sender, ScanChangedEventArgs e);
        // declare the bookpricechanged event using the bookpricechangeddelegate
        public event ScanChangedHandler ScanChanged;

        public Scan()
        {
            totalSize = 0;
            totalFiles = 0;
            OS = Tools.getOS();
        }

        public void scan()
        {
... .. ... 

            foreach (string directory in al)
            {
                currentSizeInfo = new SizeInfo();

                if (currentSizeInfo.GetDirectorySize(directory))
                {
                    OnChange();
                }
            }
        }

        protected void OnChange()
        {
            ScanChanged(this, new ScanChangedEventArgs(directory,currentSizeInfo));
        }

    }

    public class ScanChangedEventArgs : EventArgs
    {
        private SizeInfo si;
        private string directory;

        public ScanChangedEventArgs(string directory, SizeInfo si)
        {
            this.directory = directory;
            this.si = si;
        }

        public string getDirectory()
        {
            return this.directory;
        }

        public SizeInfo getSizeInfo()
        {
            return this.si;
        }
    }

 

[JurkMonkey]

If this is the case then you should be able to run your Scan() method Asyncronously as stated above. By invoking the method on the GUI thread you should no longer receive cross-threading exceptions.

http://www.ubuntu.com/

 

[zackiv31]

Sorry but because of my lack of knowledge could you be specific? Here's a code snippet:

            s = new Scan();
            scanDelegate = new Scan.ScanChangedHandler(scanChanged);
            s.ScanChanged += scanDelegate;
            s.scan(); //Isn't this run in Gui thread?  problem here is it doesn't run asynchronously. (Don't I NEED a worker thread to run it async?)  If I do it the way below, I get the cross threading issues.

            /*BackgroundWorker worker = new BackgroundWorker();
            worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
            worker.DoWork += new DoWorkEventHandler(worker_DoWork);
            worker.RunWorkerAsync();*/
        }

 

[JurkMonkey]

If you make your solution temporarily available I can look at it tonight.

http://www.ubuntu.com/