Code Review found problems

FrmMain. cs has a problem.

1. int I = 0 is set to a global constant and is not at the top of the class. It is difficult to find out if a problem occurs.

I is a common temporary variable. Setting global variables may cause confusion.

2. defined global variables are used only in one method. Too many global variables are defined.

3. Variable names and control names are ambiguous and without comments, such as the global variables defined at the top

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

Others are like timer1, timer2, l1, l2, etc...

4. There are multiple duplicates or similar code

See the following code.

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

The following methods are called multiple times without reconstruction. If the returned values are changed in the future, it is easy to confuse multiple modifications.

Void isSuccess (object iparam, object oparam) Line: in rows 138-236

Private void FrmMain_Load (object sender, EventArgs e) 465-520 rows

Private void Delete ToolStripMenuItem_Click (object sender, EventArgs e) lines-

Private void btnToParent_Click (object sender, EventArgs e) returned to the upper-level 782-842 rows

Private void ChangeListViewDisplayStyle (object sender, EventArgs e) Change the display mode of the file list in lines 867-907

Private void btnSearch_Click (object sender, EventArgs e) 1325-1377

 

In addition, the following code is used to check whether a file with the same name exists in the uploaded file, but the code is copied repeatedly in multiple methods.

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 ("You have uploaded a file" + saveName + "with the same name. Do you want to overwrite it? "," Warning ", 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 ;}}}

The following code changes the number of file bytes to KB, MB, and GB. Similar code is found in multiple places without extracting public methods.

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

The Code contains the following code:

If (UsedSpace <1000) {this. text = "Local Tax cloud disk" + 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 = "Local Tax cloud disk" + FrmLog. loginedUser. realName + "" + sumFileSize + "M/" + quotaNum ;}

The code above is slightly improved:

1. Extract "Local Tax cloud disk" + FrmLog. LoginedUser. realName + "as the variable, as shown below:

String title = "Local Tax volume" + FrmLog. LoginedUser. realName + "";

2. Use (UsedSpace/1024). toString ("f2") directly in MB.

 

5. The code is obscure and lacks comments, as shown in

 

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 ("You have uploaded a file" + saveName + "with the same name. Do you want to overwrite it? "," Warning ", 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 ;}

The intention should be to prompt the existence of a file with the same name. If you click Yes, upload the file to overwrite the file; otherwise, upload the file directly. The idea should be clear and I don't know why to compare it! (I = lvItemsList. Items. Count + 1! (I = lvItemsList. Items. Count + 1) This code is a little lame and easy to understand. It is generally I! = LvItemsList. Items. Count + 1

 

6. The code is lengthy. if else is nested multiple times, it is easy to get dizzy. We recommend that you extract the code or add return

 

/// <Summary> /// double-click to enter the folder /// </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 there is a problem here, the folder can also contain points ., {lvItemsList should be judged by itemtype. 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 = "folder") {icon = UrlImage. imageIndex (". folder "); // Add new} else {icon = UrlImage. imageIndex (System. IO. path. getExtension (lvItem. text); // Add} lvItem. imageIndex = icon; // newly added // 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 ;}}