Critique my first C#.NET application

[Conrad11]

Hi there,

This is the first C# program I have created:

Download eSync

Download eSync Source

Basically - you choose two locations, change some of the options to suit yourself and it syncs the second directory, to match the first. Great if you have your music library in two locations for instance (which is why i created the program).

Being a VB man myself it's taken some time to get used to the syntax and having to explicitly convert everything the right data types yourself, although, now I have done it, I think that's one of the many great things about C#, despite the fact it was very annoying at the beginning.

Another thing I thought was good, was the fact it doesn't take into account newlines so you could make the code fit so you didn't have to scroll horizontally.

Anywho - for anyone who has learnt/is learning VB, i strongly suggest C#. I will be using it from now on when i can.

Thanks for looking.

 

[NathanE]

Give us the source then :/

 

[Inquisitor]

Don't really have anything to sync with it at the moment, so I haven't really used it properly, but the UI seems very nicely designed

One suggestion that I'd make, though, would be to move your sync logic outside your main form. It's usually best to have only display logic in your form classes.

Give us the source then :/

I thought that and had to resort to Reflector

 

[Conrad11]

Sorry guys - here's the source:

Download eSync Source

 

[Conrad11]

One suggestion that I'd make, though, would be to move your sync logic outside your main form. It's usually best to have only display logic in your form classes.

I know what you are saying - and it makes sense - its just i don't know how i would do it.

Do you mean: create a whole new sync class with the background worker in it and handle all the sync in that class.

 

[Inquisitor]

Pretty much, though I'd keep the thread management in the form class, as threading is a UI issue.

It's generally a good idea to try and restrict form classes to presentational logic, as it makes the application a lot easier to manage and understand. Otherwise, you'll end up with loads and loads of disorganised code in your form classes (mixed up with presentational code) that you'll have to dig through if you want to change something.

 

[Conrad11]

Pretty much, though I'd keep the thread management in the form class, as threading is a UI issue.

Hmm...well this is something new to me...could you explain how you'd go about it. I need the background worker reportprogress event to update the log - so how would i keep the background worker in the form and outsource the syncing to a class? Would i have to do cross-thread calls or something?

Sorry if that makes no sense - i've been drinking.

EDIT:

Yeah, i understand, it does make sense to move the sync logic, its just i thought it's not a big program....but if it was bigger, it does make sense - thanks.

 

[Goksly]

The UI should just be used to allow the user to access the functionality of the application. I always try to design my applications in a tier design; make it so if someone wanted to replace my fancy GUI with a console or turn it into a service, they could do so pretty easily.

As Inquisitor said, only the UI thread should update the UI, but its a pretty simple problem to get around (this.InvokeRequired), so there isnt a reason not too! Basically in .Net 1.1 you could update the UI with other threads but this could lead to bugs which would pretty much be impossible to debug. In .Net 2 there is a flag you can use (InvokeRequired) to see if the currently executing thread needs to be invoked on the form. If InvokeRequired is true, all you need to do is use a delegate to the current method and invoke it on the correct thread:

private void MyMethod(IAsyncResult myData)
        {
            if (myData.IsCompleted)
            {
                if (this.InvokeRequired)
                    this.Invoke(new MethodInvoker(delegate() { MyMethod(myData); }));
                else
                {
                    // Perform action here.
                }
            }
        }

 

[SpicyDuck]

Can't run the .exe here, say's it 'encountered a problem'.

 

[Conrad11]

<snip>

Yeah that's what i meant by "cross-thread" calls....that's what I call it any ways. Thanks. I'll have a look at sorting that out later today.

Can't run the .exe here, say's it 'encountered a problem'.

Needs .NET 2. If you don't have that then it won't run. Its for windows only as well.

(It may require .NET 3 - but i don't think so.)

 

[Goksly]

I also couldnt get it to compile - said something about a certificate issue or something? I don't have anything .Net 3 or above related on my machine...

 

[Conrad11]

I also couldnt get it to compile - said something about a certificate issue or something? I don't have anything .Net 3 or above related on my machine...

Yeah - i don't know how to sort that out - its to do with a key...but i dont know how to get rid of the key.

Anyone know - its something to do with visual studios?

 

[Vai]

Yeah - i don't know how to sort that out - its to do with a key...but i dont know how to get rid of the key.

Anyone know - its something to do with visual studios?

Open the solution.
Right click eSync -> Properties
Signing
Untick "Sign the ClickOnce manifests"

 

[Dj_Jestar]

Pretty much, though I'd keep the thread management in the form class, as threading is a UI issue.

I'm strongly against this. Threading is a control issue, not a UI issue. Besides which, they are two separate behaviours - threading, and UI. Thus they should be in their own objects.

 

[Conrad11]

I was just messing around with moving the stuff to external classes and was wondering: where are the "Settings" actually stored?

I thought they might be stored in an external file, but it appears not, as there are no files created in the directory of the program, unless its stored in a different location?

Thanks.

EDIT:

Also does anyone a decent tutorial for working with databases in VS.NET 2005 (C# or VB).

EDIT Again:

Are settings "slower" to access than a variable. For instance, if i had a variable of boolean type and a settings of boolean type, would the variable be quicker to reference?

 

[Inquisitor]

I'm strongly against this. Threading is a control issue, not a UI issue. Besides which, they are two separate behaviours - threading, and UI. Thus they should be in their own objects.

I can see your point, but the reason the work is being done on a separate thread is to maintain UI responsiveness, which means it's also a UI issue. This is why background worker components exist in the System.Windows.Forms API.

I was just messing around with moving the stuff to external classes and was wondering: where are the "Settings" actually stored?

I thought they might be stored in an external file, but it appears not, as there are no files created in the directory of the program, unless its stored in a different location?

Documents and Settings\<user>\Local Settings\Application Data\<application name>

Are settings "slower" to access than a variable. For instance, if i had a variable of boolean type and a settings of boolean type, would the variable be quicker to reference?

The settings are read from the XML file on first request (individually), which will be ever-so-slightly slower, and from then on are cached in memory.

 

[pinkaardvark]

I can see your point, but the reason the work is being done on a separate thread is to maintain UI responsiveness, which means it's also a UI issue. This is why background worker components exist in the System.Windows.Forms API.

I'm sure you'll find it's there because it is a COMPONENT. For the drag and drop generation who can't read an api and write code.