代码Review发现问题

时间：2014-05-12 13:51:01 阅读：370 评论：0 收藏：0 [点我收藏+]

FrmMain.cs中存在问题

1. int i=0 设定为了全局常量且未在类顶部，出现问题时不好查找

i 属于常用临时变量，设定全局变量容易引起混乱

2.定义的全局变量但仅在一处方法中使用，定义全局变量过多

3.变量名及控件名等意义不明确又缺少注释，如顶部定义的全局变量

long length = 0;
        long loading = 0;
        private string oldPath = null;
        private int random = 1;
        private int repeat = 0;
        private string quotaNum = null;

其他类似 timer1,timer2,l1,l2等等。。。

4. 存在多处重复或相似代码

如下面一段代码

for (int i = 0; i < FrmLog.FileListOfLoginedUser.Count; i++)
{
    if ((FrmLog.FileListOfLoginedUser[i].Path) == CurrentPath)
    {
        string itemName = FrmLog.FileListOfLoginedUser[i].ItemName;
        string path = FrmLog.FileListOfLoginedUser[i].Path;
        string[] itemArr = new string[5];
        itemArr[0] = itemName;
        itemArr[1] = path;
        itemArr[2] = FrmLog.FileListOfLoginedUser[i].ItemType;
        itemArr[3] = FrmLog.FileListOfLoginedUser[i].FileSize.ToString() + "KB";
        itemArr[4] = FrmLog.FileListOfLoginedUser[i].UploadTime;
        itemNameList.Add(itemArr);
    }
}

在以下方法中多次调用而没有重构提取出来，日后返回值如有变动需要多处修改很容易混乱

void isSuccess(object iparam, object oparam) Line : 在138-236 行

private void FrmMain_Load(object sender, EventArgs e) 465-520行

private void 删除ToolStripMenuItem_Click(object sender, EventArgs e) 710-778行

private void btnToParent_Click(object sender, EventArgs e) 返回到上一级 782-842行

private void ChangeListViewDisplayStyle(object sender, EventArgs e) 改变文件列表显示方式 867-907行

private void btnSearch_Click(object sender, EventArgs e) 点击搜索时 1325-1377

另外如下面一段代码，作用是为了检测上传的文件是否存在同名文件，但是在多个方法中反复拷贝了这段代码

for (int j = 0; j < files.Length; j++)
            {
                saveName = Path.GetFileName(files[j]);
                int i = 0;
                for (i = 0; i < lvItemsList.Items.Count; i++)
                {
                    ListViewItem item = lvItemsList.Items[i];

                    if ((lvItemsList.Items[i].Text).Contains(Path.GetFileName(saveName)))
                    {

                        if (MessageBox.Show("您已上传文件" + saveName + "有同名，是否覆盖？", "警告", MessageBoxButtons.YesNo) == DialogResult.Yes)
                        {

                            repeat = 1;
                            UpLoadFile(FrmLog.ServerUrl + "/Default.aspx/", files[j], saveName, progressBar1);
                            // timer2.Enabled = true;
                            i = lvItemsList.Items.Count;
                        }
                        else
                        {
                            this.Random();
                            i = lvItemsList.Items.Count;
                        }
                    }
                }

还有下面一段代码，作用是根据文件字节数改为以KB,MB,GB等方式显示，多处存在类似代码而未提取公用方法

if (FrmLog.FileListOfLoginedUser[j].FileSize * 1024 < 1024)
                    {
                        downSize = (FrmLog.FileListOfLoginedUser[j].FileSize * 1024) + "B";
                    }
                    else if (FrmLog.FileListOfLoginedUser[j].FileSize < 1024)
                    {
                        downSize = FrmLog.FileListOfLoginedUser[j].FileSize + "KB";
                    }
                    else
                    {
                        downSize = decimal.Round(Convert.ToDecimal(FrmLog.FileListOfLoginedUser[j].FileSize / 1024), 2).ToString() + "M";
                    }

该代码存在其他处的代码如下：

if (UsedSpace < 1000)
            {

                this.Text = "地税云盘  " + FrmLog.LoginedUser.realName + "  " + UsedSpace + "KB/" + quotaNum;
            }
            else
            {
                float sumFileSize = UsedSpace / 1024;
                //decimal d = decimal.Parse(sumFileSize.ToString());
                decimal d = Convert.ToDecimal(sumFileSize);
                sumFileSize = float.Parse(decimal.Round(d, 2).ToString());
                this.Text = "地税云盘  " + FrmLog.LoginedUser.realName + "  " + sumFileSize + "M/" + quotaNum;
            }

上面代码微改进：

1.提取 "地税云盘 " + FrmLog.LoginedUser.realName + " "为变量，如下

string title="地税云盘 " + FrmLog.LoginedUser.realName + " ";

2.以MB显示直接用 (UsedSpace/1024).toString(“f2”)即可

5.代码画蛇添足晦涩难懂又欠缺注释，如

for (int j = 0; j < files.Length; j++)
            {
                saveName = Path.GetFileName(files[j]);
                int i = 0;
                for (i = 0; i < lvItemsList.Items.Count; i++)
                {
                    ListViewItem item = lvItemsList.Items[i];

                    if ((lvItemsList.Items[i].Text).Contains(Path.GetFileName(saveName)))
                    {

                        if (MessageBox.Show("您已上传文件" + saveName + "有同名，是否覆盖？", "警告", MessageBoxButtons.YesNo) == DialogResult.Yes)
                        {

                            repeat = 1;
                            UpLoadFile(FrmLog.ServerUrl + "/Default.aspx/", files[j], saveName, progressBar1);
                            // timer2.Enabled = true;
                            i = lvItemsList.Items.Count;
                        }
                        else
                        {
                            this.Random();
                            i = lvItemsList.Items.Count;
                        }
                    }
                }
                if (!(i == lvItemsList.Items.Count + 1))
                {
                    UpLoadFile(FrmLog.ServerUrl + "/Default.aspx/", files[j], saveName, progressBar1);
                    //  timer2.Enabled = true;
                    i = lvItemsList.Items.Count;
                }

意图应该是存在同名文件进行提示，点Yes则上传覆盖文件，否则直接上传，这样应该思路很清晰，不知为何还要比较!(i == lvItemsList.Items.Count + 1)，另外!(i == lvItemsList.Items.Count + 1) 这种代码写法有些蹩脚容易让人费解，一般都是 i!=lvItemsList.Items.Count + 1

6. 代码冗长，多次嵌套if else 容易让人看晕，建议提取出方法或添加return

/// <summary>
        /// 双击进入文件夹
        /// </summary>
        private void lvItemsList_MouseDoubleClick(object sender, MouseEventArgs e)
        {
            this.isFind = false;
            ListViewHitTestInfo info = lvItemsList.HitTest(e.X, e.Y);

            if (info.Item == null) return;
            lvItemsList.LargeImageList = UrlImage.SmallImageList;

            if (!(info.Item.Text.Contains(".")))//todo 此处存在问题，文件夹也可包含点 .，应以itemtype判断
            {
                lvItemsList.Items.Clear();
                CurrentPath = info.Item.Tag.ToString() + "/" + info.Item.Text;
                // CurrentPath = info.Item.Tag.ToString() + "/" + info.Item.Text;
                FrmLog.FileListOfLoginedUser = this.GetFileList();
                oldPath = info.Item.Tag.ToString();

                if (FrmLog.FileListOfLoginedUser == null)
                {
                    this.lblCurPath.Text = CurrentPath;
                    return;
                }
                for (int i = 0; i < FrmLog.FileListOfLoginedUser.Count; i++)
                {
                    if (FrmLog.FileListOfLoginedUser[i].Path == CurrentPath)
                    {
                        ListViewItem lvItem = new ListViewItem();
                        lvItem.Text = FrmLog.FileListOfLoginedUser[i].ItemName;
                        lvItem.Tag = FrmLog.FileListOfLoginedUser[i].Path;
                        if (switchViews == "1")
                        {
                            /* if (!(lvItem.Text.Contains(".")))
                             {
                                 lvItem.ImageIndex = 0;
                             }
                             else
                             {
                                 int icon = UrlImage.ImageIndex(System.IO.Path.GetExtension(lvItem.Text));
                                 lvItem.ImageIndex = icon;
                             }*/
                            int icon = 0;
                            if (FrmLog.FileListOfLoginedUser[i].ItemType == "文件夹")
                            {
                                icon = UrlImage.ImageIndex(".folder");//新加
                            }
                            else
                            {
                                icon = UrlImage.ImageIndex(System.IO.Path.GetExtension(lvItem.Text));//新加
                            }
                            lvItem.ImageIndex = icon;//新加

                            //int icon = UrlImage.ImageIndex(System.IO.Path.GetExtension(lvItem.Text));
                            //lvItem.ImageIndex = icon;
                            //  oldPath = FrmLog.FileListOfLoginedUser[i].Path;

                            this.lvItemsList.Items.Add(lvItem);
                        }
                        else
                        {
                            lvItem.SubItems.Add(FrmLog.FileListOfLoginedUser[i].ItemType);
                            if (!(lvItem.Text.Contains(".")))
                            {
                                lvItem.SubItems.Add("");
                            }
                            else
                            {
                                lvItem.SubItems.Add(FrmLog.FileListOfLoginedUser[i].FileSize.ToString() + "KB");
                            }

                            //   lvItem.SubItems.Add(FrmLog.FileListOfLoginedUser[i].FileSize.ToString());
                            lvItem.SubItems.Add(FrmLog.FileListOfLoginedUser[i].UploadTime);
                            this.lvItemsList.Items.Add(lvItem);

                        }

                    }
                    else
                    {
                        oldPath = info.Item.Tag.ToString();
                    }
                }
                this.lblCurPath.Text = CurrentPath;
            }
        }

代码Review发现问题,布布扣,bubuko.com

代码Review发现问题

标签：style blog class code java tar

原文地址：http://www.cnblogs.com/s1ihome/p/3720275.html

踩

(0)

评论一句话评论（0）

分享档案

更多>

2021年07月29日 (22)
2021年07月28日 (40)
2021年07月27日 (32)
2021年07月26日 (79)
2021年07月23日 (29)
2021年07月22日 (30)
2021年07月21日 (42)
2021年07月20日 (16)
2021年07月19日 (90)
2021年07月16日 (35)

周排行