代码Review发现问题
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;}}}還有下面一段代碼，作用是根據文件字節(jié)數改為以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;}}轉載于:https://www.cnblogs.com/s1ihome/p/3720275.html
總結
以上是生活随笔為你收集整理的代码Review发现问题的全部內容，希望文章能夠幫你解決所遇到的問題。
- 上一篇： Careercup - Google面试
- 下一篇： boost常用记录