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

还有下面一段代码,作用是根据文件字节数改为以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;
}
}

代码Review发现问题,布布扣,bubuko.com

时间: 2024-09-29 07:55:51

代码Review发现问题的相关文章

代码review

对于代码review个人也有些小小的看法: 1.首先我觉得我们所有开发人员要弄明白 现在Code Review 的目的 ,凡事不弄明白目的,无法做好完成一件事情,个人觉得有以下一些目的: a)可以在项目早期就能够发现代码中的BUG ,提测后可以尽快的释放开发资源:b)同时可以达到知识共享 ,避免我们所有开发人员犯一些很常见,很普通低级的错误 :c)保证项目组人员的良好沟通 ,项目的代码更容易维护 大家还有希望补充上 2.Code Review 很容易变得没有意义或是流于形式,进入 Code Re

由学习《软件设计重构》所想到的代码review(一)

前言 对于一个程序员来讲如何来最直接的来衡量他的技术能力和产出呢?我想最直观的作法是看他的代码编写能力,就拿我经常接触的一些程序员来看,他们买了很多技术重构类书籍,但是看完后代码编写能力并没有显著提高.有人说可以用代码review工具啊,但是像市面上的这些代码review工具,只能帮助我们解决表面的bug和规范点,还无法帮助我们发现更深层次的设计问题. 下面我将结合<软件设计重构>这本书谈谈在进行代码review的时候,需要关注的哪些点. 一.技术债务 何为技术债务? 技术债务是有意或无意的做

代码Review那些事

本篇推文是以前同事做分享的时候的ppt,这里我整理出来分享给大家 什么是代码Review? 代码review是指在软件开发过程中,通过对源代码进行系统性检查来确认代码实现的质量保证机制 为什么不做代码Review? ?业务需求大,工作时间紧张 项目小,协作的人少,没必要 为什么要做代码Review? 提高代码质量,提升自身水平 及早发现潜在缺陷与BUG,降低事故成本 促进团队内部知识共享,提高团队整体水平 保证项目组人员的良好沟通 避免开发人员犯一些很常见,很普通的错误 总而言之目的是查找系统缺

由学习《软件设计重构》所想到的代码review(二)

我们接第一篇由学习<软件设计重构>所想到的代码review(一) 来继续说明在代码review中,有哪些属于"层次结构"中的坏味道. 注:通过上图咱们看到了在层次结构中有九大问题点,咱们就从中找出三个典型的问题点给与分析和解释. 一.缺失的层次结构 问题点: public Insets getBorderInsets(Component c, Insets insets) { if(c instanceof AbstractButton) { margin = ((Abst

Git——刚刚提交的代码,发现写错了怎么办?

刚刚提交的代码,发现写错了怎么办? 刚提交了一个代码,发现有几个字写错了: 怎么修复? 当场再写一个修复这几个错别字的 commit ?可以是可以,不过还有一个更加优雅和简单的解决方 法: commit --amend . "amend" 是「修正」的意思.在提交时,如果加上 --amend 参数,Git 不会在当前 commit 上增加 commit ,而是会把当前 commit 里的内容和暂存区(stageing area)里的内容合并起来后创建一个新的 commit ,用这个新的

如何进行代码review

代码review是质量保证(QA)的手段之一,但不是用来替代测试的,特别是自测. 一个软件项目的质量定义并不是代码review的职责,换句话说,良好的质量定义是代码review发挥效果的必要前提. 代码review到底要review哪些东西? 代码风格 代码结构(架构与设计) 核心逻辑 想要通过代码review来检测每一行代码,并确保检查出所有问题是不可能的,它更侧重于处理核心且明显的问题. 谁来review? 这个要看开发组采取的review形式,一般分为独立review和集中review,前

Gerrit代码Review入门实战

代码审核(Code Review)是软件研发质量保障机制中非常重要的一环,但在实际项目执行过程中,却因为种种原因被Delay甚至是忽略.在实践中,给大家推荐一款免费.开放源代码的代码审查软件Gerrit. 1.Why Code Review Code Review是什么 Code Review最直观的解释即看代码.常规的做法为自己看,有时代码逻辑问题可能自己看不出来,需要找同事一起看,在大家知识体系相对平均的情况下可能需要花钱专门的公司帮助查看. Code Review需要看哪些?对于刚入职场或

第一次代码review所了解到的问题(非本人)

需求描述:从数据库中导出一张报表,报表的表头比较复杂,给出开始时间和结束时间,导出在这段区间内的所有的数据 原来的代码:自定义工具类,从空白表开始写,定义了一系列的数组表头,然后先写表头,再按条件查询数据写入文件中,直接导出到response的输出流中完成下载. 指出的问题:从头开写太浪费时间了,并且表头的宽度高度颜色等样式比较复杂,会占用大量代码,不如在工程目录下放置一个模板文件,里面只有表头数据, 在每次下载的时候,我们直接复制一份这个文件,用uuid来唯一命名,然后将导出的数据写入到复制的

android项目《刷刷手环》代码review

很多程序很多程序猿可能会和我一样,当公司开发项目时,完成功能是第一位,从而总会出现这样的话,这里应该可以写的更好,下版本再说.最近项目接近尾声,感觉需要重新审视一下这个项目,这应该是提升自己和优化项目的最好的办法之一. 废话结束.... 一.分享 方案一:直接使用友盟分享 http://dev.umeng.com/social/android/quick-integration 方案二:分别调用各个平台的sdk 1.分享朋友圈和微信 private void regToWx() { api =