`审核人：孙雨珩` `被审核代码负责人：欧阳德才` `代码地址：https://192.168.1.240:8443/svn/repos/jizhen-fe` # 技侦指挥大屏代码审查 #### 1、使用jquery的show&hide控制元素是否显示(已修改) 在调用jquery元素的hide方法时，jquery还需要去获取元素原始display属性的值并进行缓存，然后再恢复展示时又要去缓存里获取原始属性值再恢复，太耗性能 解决方案：使用一个css class控制元素是否展示 #### 2、d3操作时对于enter下的元素和update下的元素重复进行样式更新(已修改) enter后append的元素实际上会加回到update对应的数组下，样式更新只需要针对update就好 解决方案： ``` var selection = d3.selectAll('xxx').data(data) // 这里只做新增，不更新样式 var enterSelection = selection .enter() enterSelection.append('xxx') // 无用的元素直接移除 selection .exit() .remove() // 现在这个selection不仅包含通过selectAll选中的元素，还包含enter进来的元素，统一更新即可 selection ``` #### 3、right.js的drawLiquidFillGauge方法创建了一个无用的对象(已修改) 在方法末尾创建了一个GaugeUpdater对象，每一个这个对象每次还会创建一个带闭包的函数，但最后根本没有使用 解决方案： 确认确实没有用后将创建GaugeUpdater对象的逻辑删除 #### 4. 在调用map.js的drawSymbol函数时，在一定入参下会创建一个无法结束的定时任务(已修改) 当入参isSmallMap为真值时，会在函数末尾创建一个无法结束的定时任务，当这个函数被重复调用时会出现更多的定时任务 解决方案： ``` function draw() { var cancelled = false setTimeout(function job () { // 定时任务逻辑写在这里 if (cancelled) { return } setTimeout(job, 1000) }, 1000) return { cancel: function () { cancelled = false } } } ``` #### 5.在map.js的drawSymbol函数中，创建了三层嵌套的setTimeout(已修改) 解决方案： 第一层回调函数可以省略，直接根据transitionItems的长度判断是否要创建第二层setTimeout #### 6.在map.js的drawSymbol函数中，使用了arguments.callee arguments的.callee已经被废弃，不允许使用 ``` function () { setTimeout(arguments.callee, 2000); } ``` 解决方案： ``` // 不使用匿名函数即可 function job() { setTimeout(job, 2000); } ``` #### 7.在调用right.js的drawLiquidFillGauge函数时，会创建一个无法结束的定时任务(已修改) 当这个函数被重复调用时会出现更多的定时任务 解决方案：参照第4点 #### 9.在map.js的initMap函数中，在为区块绑定事件时创建了无用的匿名函数(已修改) 这创建了一个没有闭包的匿名函数，并且匿名函数中的写法还创建了一个闭包 ``` xxx.on('mouseout', function() { mouseOut() }) ``` 解决方案： ``` xxx.on('mouseout', mouseOut) ``` #### 10.在map.js的victoryCallback函数中，采用循环数组的方式查找和地区id相匹配的中心点位置(已修改) 如果用来查找id在数组的末尾，相当于整个数组都要循环一遍。而且这里不仅查找小地图中的中心点，还查找大地图的中心点，在最坏情况下要循环完两个数组 ``` for (j = 0, length = smallMapCentroids.length; j < length; j++) { if (smallMapCentroids[j].id == kuaibaoData.areaId) { kuaibaoData.geoCoord = smallMapCentroids[j].centroid mapNode = '.small-map' isSmallMap = true break } } ``` 解决方案：中心点信息不用数组管理，而是用一个对象以区域id作为索引 ``` var feature = { id: xxx, centroids: xxx } var smallMapCentroids = {} smallMapCentroids[feature.id] = feature // 这样就不用循环了 var searchId = yyy var feature = smallMapCentroids[searchId] ``` #### 10.在map.js的victoryCallback函数中，每次都通过jquery元素的.is(':hidden')检查一个元素是否展示(已修改) .is(':hidden')底层实现不仅会尝试通过getComputedStyle查询元素是否存在display:none的css，还会尝试检查元素是否在当前文档中，影响性能 解决方案：对于元素显示或隐藏，使用class控制。在检查元素状态时，通过检查元素是否有这个class即可 #### 11.在victoryAnimation.js的drawPoint方法中，使用jQuery查找了容器下的svg，但下面并没有使用这个元素(已修改) ``` // 这个元素在下面并没有被使用 var container = $(id).find('svg') ``` 解决方案：确认无用后，将查询语句删除 #### 12.在victoryAnimation.js的drawPoint方法中，创建了无用函数remove(已修改) ``` { drawPoint: function () { // 这个函数没有被使用 function remove() { d3.selectAll('.vicoryTooltip').remove() d3.selectAll('.carouselPoint').remove() } } } ``` 解决方案：确认无用后，将函数删除 #### 13.在victoryAnimation.js中的drawPoint方法中，用d3为新建元素绑定了data(已修改) **这里使用的data是一个对象，而不是数组。所以可以得到以下结论：** 1. 新建时，创建元素的数量跟数据长度无关，是固定只有一个。不需要根据数据长度增减元素。 2. 更新元素属性时，因为数据只有一个，对应元素也只有一个。不需要通过数据绑定的方式，为不同数据对应的元素分别更新元素属性。直接使用data更新元素属性即可。 所以说，在只有一个数据的情况下，根本不需要绑定data。 这个data本身带有的数据较多，在渲染完毕就可以丢弃的情况下**不应该绑数据**。 因为如果通过d3的selection.data方法绑定数据后，会在选中的元素上创建__data__属性来保存这个数据，如果通过这个元素select了后代元素，那么后代元素也会有这个__data__属性，导致data无法被释放。 解决方案： ``` function createImage(container) { // 举个例子，该函数只在新建时调用，所以直接创建需要的image元素即可 container .append('image') .classed('ratate-img', true) } ``` ``` function updateImage(container, data) { // 更新时，直接修改元素对应属性 container .select('.ratate-img') .attr('xxx', data.yyy) // 甚至不需要用以下这种写法 // .attr('xxx', function () { return data.yyy }) } ``` #### 14.在victoryAnimation.js中，暴露了全局变量markData，coor，config，carouselPoint(已修改) 看代码，作者的意图主要是在drawPoint时，为相关用于新建元素的函数，以及tooltip函数，提供元素容器和数据。 这些信息应该只保留在drawPoint函数执行过程中，而不是暴露到全局，导致了内存浪费 ``` function addPoint() { // 没有通过入参，而是使用的全局变量 carouselPoint markData } ``` ``` function(position, victoryId, isSmallMap, markId) { // 没有通过入参，而是使用的全局变量 markData } ``` 解决方案： 在调用对应函数时，将相关数据作为入参传入即可，并不需要暴露到全局 ``` function addPoint(carouselPoint, markData) { // ... } ``` ``` function(markData, victoryId, isSmallMap) { // 原函数签名要求position，实际上就是markData的一个子属性 // 原函数签名中的markId在函数执行过程中没有用到，可以去除 var position = markData.coor } ```