Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I would like to know how I could improve my C# project. I'm not really sure whether or not I'm allowed to just post a link to the Google Code project here, so sorry if it's not allowed, I'll edit my question if someone tells me so.

http://code.google.com/p/simple-irc/source/browse/#svn%2Ftrunk

The reason I'm posting the entire project is because I would like to know if there's anything I can improve coding-wise (coding conventions I'm missing or misusing?), structural-wise (should anything be restructured, do I need to split it up in different classes etc?) and document-wise (how should I document my variables, methods, etc?).

Edit: Maybe I should explain a few things to save anyone who reads this some time. The project is split up in three "modules". The UI part of the application is in the "Source" folder, the core IRC library is in the "Library" folder and any plugins (that the UI module may load) are in the "Plugins" folder.

Edit2: I'll edit my question as a reply to the replies, this is a picture of the application: http://matthias.van-eeghem.com/data/simpleIRC.png

This is a small bit of code (as I apparently have to include that according to the FAQ):

/// <summary>
/// Called when the client receives the your hostName is event
/// </summary>
/// <param name="serverIndex">The server index</param>
/// <param name="hostName">The displayed hostName</param>
private void OnDisplayedHostReceived(int serverIndex, string hostName)
{
    string Output = "Your displayed hostName is: " + hostName + "\r";
    Core.ServerList[serverIndex].Logger.Log(Output);
    ServerWindow.List[serverIndex].SendText(Output);

    foreach (PluginInterface Plugin in MainWindow.PluginList) if(Plugin != null) Plugin.OnDisplayedHostReceived(serverIndex, hostName);
}

/// <summary>
/// Called when the server sends all the commands supported to the client
/// </summary>
/// <param name="serverIndex">The index of the server</param>
/// <param name="commandsSupported">The list of the commands supported</param>
private void OnCommandsSupportedReceived(int serverIndex, string commandsSupported)
{
    string Output = "Supported: " + commandsSupported + "\r";
    Core.ServerList[serverIndex].Logger.Log(Output);
    ServerWindow.List[serverIndex].SendText(Output);

    foreach (PluginInterface Plugin in MainWindow.PluginList) if(Plugin != null) Plugin.OnCommandsSupportedReceived(serverIndex, commandsSupported);
}

/// <summary>
/// Called when the client times out from the server
/// </summary>
/// <param name="serverIndex">The serverIndex</param>
private void OnClientTimeout(int serverIndex)
{
    Server Server = Core.ServerList[serverIndex];
    // Could not resolve hostName 
    string Output = "* Lost connection to " + Server.Hostname + "\r";
    Core.ServerList[serverIndex].Logger.Log(Output);
    ServerWindow.List[serverIndex].SendText(Output);

    Output = "* Trying to reconnect..\r";
    Core.ServerList[serverIndex].Logger.Log(Output);
    ServerWindow.List[serverIndex].SendText(Output);

    // Loop through all channels
    for (int i = 0; i < Core.ChannelList.Count; i++)
    {
        Channel Channel = Core.ChannelList[i];
        if(Channel != null && Channel.Server != null)
        {
            if (Channel.Server.Index == serverIndex)
            {
                Output = "* Lost connection to " + Server.Hostname + "\r";
                Core.ServerList[serverIndex].Logger.Log(Output);
                ChannelWindow.List[i].SendText(Output);
            }
        }
    }

    // Try reconnecting
    Server.Connect();

    foreach (PluginInterface Plugin in MainWindow.PluginList) if(Plugin != null) Plugin.OnClientTimeout(serverIndex);
}

I'd love any feedback for this code specifically but I'd also appreciate any comments in regards to the entire project structure etc.

share|improve this question
    
Can we see some pics? –  Bryan Dunsmore Jun 3 '12 at 21:29
add comment

1 Answer

One thing I noticed at a glance is that the main window code-behind is doing a lot of work that isn't really UI-focused. For example, the code to handle loading plugins from external DLLs is not something I would expect a Window subclass to do. I would highly recommend restructuring the application to utilize pattern that helps you to break things up into more single-purpose classes.

A good example of a pattern that I use often is Model-View-Presenter, or MVP. A more common pattern in WPF projects is Model-View-ViewModel, or MVVM.

The separation of view logic from business logic makes the application more modular, allowing you to swap out components more easily. For example, if you decide later that a different UI technology (e.g., HTML/Javascript Metro) is more to your liking, you just need to swap out the view.

share|improve this answer
    
Thank you for your time! I'll do the coding as soon as I get some time on my hands. –  Matthias Van Eeghem Jun 4 '12 at 20:18
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.