保持主要形式的整洁

Question

这不是关于编码本身的问题，而是关于代码结构的良好实践的问题。我目前正在构建一个WinForms应用程序，几个小时后，我的主表单包含130行代码。这可能不是很多，但这只包括事件处理-因为我试图避免这种确切的情况，通过对基本上所有的单独的类文件…但是现在所有的控件和事件让我的主代码很难阅读。

现在这是一个话题，我可以找到令人惊讶的少，我有一些想法，如何解决这个问题，如创建自定义控件和分裂成大的部分形式。有没有最好的做法呢?当80%的用户交互发生在主表单时，你如何保持主表单的整洁?还有，你能推荐一个关于如何构建项目(不是代码)的基本准则吗?

(希望这是个有效的问题)

谢谢!

编辑:我决定添加代码。看到多余的了吗?

public partial class MainForm : Form
{
    string currentFilter = "all";
    public MainForm()
    {
        InitializeComponent();
    }
    private void MainForm_Load(object sender, System.EventArgs e)
    {
        RefreshGenres();
        RefreshMovies(currentFilter);
    }
    private void addToolStripMenuItem_Click(object sender, System.EventArgs e)
    {
        var fAdd = new AddNewForm();
        fAdd.SetDesktopLocation(MousePosition.X, MousePosition.Y);
        fAdd.ShowDialog();
    }
    private void refreshToolStripMenuItem_Click(object sender, System.EventArgs e)
    {
        RefreshGenres();
        RefreshMovies(currentFilter);
    }
    private void addCategoryToolStripMenuItem_Click(object sender, System.EventArgs e)
    {
        tvCategories.Nodes.Add(new TreeNode("category"));
        tvCategories.Nodes[tvCategories.Nodes.Count - 1].BeginEdit();
    }
    private void tvCategories_AfterLabelEdit(object sender, NodeLabelEditEventArgs e)
    {
        var genre = e.Label;
        var writer = new Writer();
        writer.AddGenre(e.Label);
    }

    private void everythingToolStripMenuItem_Click(object sender, System.EventArgs e)
    {
        EraseData("all");
    }
    private void clearGenres_Click(object sender, System.EventArgs e)
    {
        EraseData("genres");
    }
    private void clearMovies_Click(object sender, System.EventArgs e)
    {
        EraseData("movies");
    }
    private void EraseData(string eraseThis)
    {
        DialogResult r = MessageBox.Show("Are you sure?nLost data can NOT be retrieved.", "Clear Data", MessageBoxButtons.YesNo, MessageBoxIcon.Warning);
        var cmdText = "";
        if (r == DialogResult.Yes)
        {
            switch (eraseThis)
            {
                case "all":
                    cmdText = "TRUNCATE TABLE MOVIES GENRES";
                    break;
                case "movies":
                    cmdText = "TRUNCATE TABLE MOVIES";
                    break;
                case "genres":
                    cmdText = "TRUNCATE TABLE GENRES";
                    break;
            }
            conn.Open();
            using (var cmd = conn.CreateCommand())
            {
                cmd.CommandText = cmdText;
                cmd.ExecuteNonQuery();
            }
            conn.Close();
        }
    }

    private void RefreshGenres()
    {
        tvCategories.Nodes.Clear();
        var reader = new Reader();
        var genres = reader.GetGenreList();
        foreach (string str in genres)
        {
            tvCategories.Nodes.Add(str);
        }
    }
    private void RefreshMovies(string filter)
    {
        lvMovies.Items.Clear();
        var reader = new Reader();
        var movies = reader.GetMovieList(filter);
        foreach (ListViewItem item in movies)
        {
            lvMovies.Items.Add(item);
        }
        reader.conn.Close();
    }
    private void tvCategories_NodeMouseClick(object sender, TreeNodeMouseClickEventArgs e)
    {
        currentFilter = e.Node.Text;
        RefreshMovies(currentFilter);
    }
}

Answer 1

我认为这里的问题不是你的代码长度，而是你混淆了许多不同的事情，你没有以一致的方式处理你的事件。以下是我想修改的主要内容:

EraseData:这个方法打开并使用一个数据库连接，这是你通常应该避免的事情(改变DB中的一些东西会导致你修改UI代码)，你的WinForm代码不应该知道它暴露的数据来自哪里。在这里，最好向Writer类添加一个EraseData。

RefreshMovies:它与refreshgenre非常不同，即使这两个方法本质上做同样的事情:它们获得添加到一些UI控件的数据列表。refreshgenre看起来不错，但是查看RefreshMovies，您会立即看到您正在关闭一个在此方法中未打开的连接。也许你在Reader类中打开了它，但你也应该在这里关闭它。实际上，conn最好是私有的(请记住，UI不需要知道数据是来自数据库、文本文件还是用户输入)。此外，GetGenreList (Reader)返回字符串列表，这没问题，但GetMovieList返回ListViewItem列表，这不好，因为这与你特定的UI实现密切相关。这意味着您的Reader的实现不能在WPF或web应用程序中使用。ListViewItems应该在RefreshMovies中使用从Reader获得的纯数据创建。