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; } }