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