博客
关于我
强烈建议你试试无所不能的chatGPT,快点击我
代码Review发现问题
阅读量:5318 次
发布时间:2019-06-14

本文共 9248 字,大约阅读时间需要 30 分钟。

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

 

///         /// 双击进入文件夹        ///         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;            }        }

转载于:https://www.cnblogs.com/s1ihome/p/3720275.html

你可能感兴趣的文章