Skip to content

Commit

Permalink
收藏页
Browse files Browse the repository at this point in the history
  • Loading branch information
Gavin-YYC committed Aug 5, 2016
1 parent 86080cc commit d00ecbd
Show file tree
Hide file tree
Showing 4 changed files with 733 additions and 0 deletions.
151 changes: 151 additions & 0 deletions yangyoucun/h5-collection/collection.model.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/**
* 收藏页 -- 模型
* @author by Gavin 2016-7-28
*/
var $ = require('home:widget/ui/zepto/zepto.js');
var B = require('home:widget/ui/backbone/backbone.js');

var Model = B.Model.extend({
defaults: {
collections: null, // 收藏数据
cateList: null, // 分类列表
editable: null, // 编辑按钮是否可点击

This comment has been minimized.

Copy link
@leeleslie7

leeleslie7 Aug 8, 2016

Contributor

从上下文来看editable是一个布尔值,所以初始化的值有些问题,并且命名不够规范,应为editAble

This comment has been minimized.

Copy link
@mingxuWang

mingxuWang Aug 8, 2016

Contributor

editable一个单词,可编辑的。

editStatus: false, // 当前商品的编辑状态 true 表示正在编辑,false表示不是编辑状态
checkedNum: 0, // 选中商品的数量
deleteList: null, // 删除商品列表
errMsg: '', // 错误信息
deleteStatus: null, // 删除商品返回状态
maxCount: 0, // 收藏商品的总数

This comment has been minimized.

Copy link
@mingxuWang

mingxuWang Aug 8, 2016

Contributor

收藏商品总数用maxCount是否合适,是不是collectionCounts更好?

This comment has been minimized.

Copy link
@jiaqian06

jiaqian06 Aug 12, 2016

总数用collectSum呢?

isEnd: false // 翻页,是否到头了
},

// 错误码统一管理
ERROR_CODE: {
0: '返回数据格式错误',
1: '数据返回异常',
2: '数据请求失败'
},

This comment has been minimized.

Copy link
@hongdou0216

hongdou0216 Aug 9, 2016

Contributor

这个为什么不是后端维护

/**
* 当页面初始化时,立即请求数据
*/
initialize: function () {

This comment has been minimized.

Copy link
@leeleslie7

leeleslie7 Aug 8, 2016

Contributor

initialize函数有两个问题:(1)定义了这个函数但是并没有使用 (2)根据getList函数的定义来看,是要接受一个参数opts的,但是在getList函数中并没有对参数缺省时做判断

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 8, 2016

Author Contributor

这是Backbone的语法,当创建Model实例的时候,会自动执行initialize方法,所以不用我手动执行,Backbone会自动执行,第二点,getList方法可以接受参数也可以不接受,因为在getList方法中我第一步进行了formatData处理,该处理会先设定一个默认值,然后通过extend方法进行参数合并,所以,对于默认不要参数的getList方法,直接使用默认参数就可以了,其他有参数的行为,我只要传入相应参数即可。

this.getList();
},

/**
* 获取收藏商品列表
* @param {opts} object
* @param {opts.catid} number 分类ID
* @param {opts.pn} number 起始页
* @param {opts.rn} number 结束页
*/
getList: function ( opts ) {
var _opts = this.formatData( opts );
this.fetch({
url: '/collect/sku/getlist',
data: _opts,
datatype: 'JSON',
timeout: 20000,
success: function ( _, data ) {

This comment has been minimized.

Copy link
@leeleslie7

leeleslie7 Aug 5, 2016

Contributor
  • 这个变量这样有什么意义吗?

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 5, 2016

Author Contributor

this.fetch是backbone提供的一个fetch方法,执行完fetch方法后会把当前model作为第一个参数传给回调函数,所以这个_指的的当前的model,不过我这样写确实不符合规范,容易与underscore的用法产生误解,以后我会注意的。(之前看到有人这么用于是就跟着用了。。。)

This comment has been minimized.

Copy link
@1d88

1d88 Aug 5, 2016

考虑到一个代码的维护性上,我感觉应该把URL这种常量,提成变量放在文件的开始或者结束,以后涉及接口的查看和修改就很方便了

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 8, 2016

Author Contributor

之前也这样想过,后来就放到各自的fetch里面了,主要是因为个人觉得该页面接口不多,model代码本身就不多,接口与请求方法相互对应,就没有单独存放管理,以后会进行统一管理

if ( data && data.ret === 0 ) {

This comment has been minimized.

Copy link
@brambledn

brambledn Aug 9, 2016

后端传数据大多数都是字符串,是不是不适宜用===?

if ( data.content && data.content.sku_list ) {
var oldCollect = _.get('collections');
if ( oldCollect === null ) oldCollect = [];

This comment has been minimized.

Copy link
@hongdou0216

hongdou0216 Aug 9, 2016

Contributor

这样不规范的写法,需不需要改

This comment has been minimized.

Copy link
@shishuo119

shishuo119 Aug 9, 2016

collectionsdefaults中设为[]是不是这里的代码就不用做判断啦,就可以去掉54行这句判断啦。

var Model = B.Model.extend({
  defaults: {
  collections: [],  // 收藏数据
……

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

恩,这样写主要是能够获取到第一次的change,如果刚开始是[],然后获取的数据也是[],那么就不会触发change事件,页面就不会render,因为即使获取的是空数据,页面也是要生成“数据为空”的提示的,所以这里便做了这样的处理。也有其他的方式,比如,初始化的时候显示空数据,数据加载的时候把空数据隐藏等,这里没有选择,于是就在model里处理了。

_.set({
// 每次请求的数据都追加到原来的数组上
collections: oldCollect.concat(data.content.sku_list),
editable: data.content.total > 0 ? true : false,

This comment has been minimized.

Copy link
@DahaiJarvis

DahaiJarvis Aug 8, 2016

editable 后面直接 data.content.total > 0就可以了吧

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 8, 2016

Author Contributor

get新技能,平时写的时候没想到这点

cateList: data.content.cat_list,
maxCount: data.content.total
});

This comment has been minimized.

Copy link
@1d88

1d88 Aug 5, 2016

data.content 多次调用,可否将其保存到局部变量

_.setIsEnd(); // 是否滚动到头了
} else {
_.set('errMsg', _.ERROR_CODE[0]);
}
} else {
_.set('errMsg', _.ERROR_CODE[1]);
}
},
error: function ( _, err ) {
_.set('errMsg', _.ERROR_CODE[2]);

This comment has been minimized.

Copy link
@sunzhaoye

sunzhaoye Aug 8, 2016

返回异常信息提示应取后端返回的信息,前端设置默认值,如果后端没有返回,做异常处理用。
var msg = data.msg || _.ERROR_CODE[2];
_.set('errMsg', msg);

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

恩,这个之前没考虑到,之前我的想法是给用户的提示是友好的,用户可以理解,而供我们排查的错误提示是清晰明确的,之前我有想是不是前端应该有一个完整并且友好的用户提示,这个提示是在后端返回的基础上包装了一层,一方面用户可理解另一方面我们能知道问题的大体方向,排查的时候看下接口就知道什么问题了。

}
});
},

/**
* 滚动时请求数据,判断是否已经没有数据了
*/
setIsEnd: function () {
var currentCount = this.get('collections').length;
var maxCount = this.get('maxCount');

This comment has been minimized.

Copy link
@shishuo119

shishuo119 Aug 9, 2016

currentCountmaxCount在这个函数中只用了一次,直接用,会不会更好呢?

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

个人感觉直接用与先定义没有什么好不好,是一个习惯吧,我习惯先获取在使用,代码上有点啰嗦,感觉比较好理解一些,比如这样写:this.set('isEnd', this.get('collections').length >= this.get('maxCount'));,我看上去可能需要翻译一下。。。我代码上还是偏于流程式。

if ( currentCount >= maxCount ) {
this.set('isEnd', true);
}

This comment has been minimized.

Copy link
@mingxuWang

mingxuWang Aug 8, 2016

Contributor

有没有这样的场景,用户访问某一类收藏列表的时候已经请求了全部数据,这时候isEnd值被设为了true;当再去请求其他类型的收藏列表时,这时的isEnd是否还是true?页面应该是没有进行视图切换,因此没有进行初始化吧?这样是否就会出现无法继续加载的情况?

更新下,在看到后面 updateCate方法时有手动设置this.model.set({isEnd:false});

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 8, 2016

Author Contributor

在updateCate的时候,已经重新设置了false,详情见174行

},

/**
* 格式化数据
* @param from this.getList()
*/
pn: 0,
formatData: function ( opts ) {
var defaults = {
catid: '',
pn: this.pn,
rn: 12
};
var newOpt = $.extend({}, defaults, opts);
return newOpt;

This comment has been minimized.

Copy link
@shishuo119

shishuo119 Aug 9, 2016

这里不用声明变量,直接返回可好?
return $.extend({}, defaults, opts);

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

恩,这样更好,少些一行代码

},

/**
* 切换商品编辑状态
*/
changeEditStatus: function () {
this.set('editStatus', !this.get('editStatus'));
},

/**
* 更新选中收藏商品的数量,同时更新删除商品列表
* @param {action} string 'add' or 'sub' 编辑收藏品的动作
* @param {skuId} number 商品sku_id
*/
updateNum: function ( action, skuId ) {
if ( !action || !skuId ) return;
var nowNum = +(this.get('checkedNum'));

This comment has been minimized.

Copy link
@brambledn

brambledn Aug 9, 2016

+号 是为了变成整数吗?需要吗?

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

看你这么晚了还这么辛苦。。唉,甚是感动啊。。。这么做只是为了安全一点,因为设置这个值的地方比较多,这里需要进行加减运算,必须是数值,所以为了安全一点,就加个+号。

var deleteList = this.get('deleteList') || [];
if ( action == 'add' ) {
deleteList.push( skuId );
this.set({checkedNum: nowNum + 1, deleteList: deleteList});
} else if ( action == 'sub' ) {
deleteList.splice(deleteList.indexOf(skuId), 1);

This comment has been minimized.

Copy link
@hongdou0216

hongdou0216 Aug 9, 2016

Contributor

deleteList.pop();

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

豆姐,我不知道删除的是哪一个呀,我可能删除数组中的随机一个,但是pop()只能删除最后一个啊

this.set({checkedNum: nowNum - 1, deleteList: deleteList});
}
},

This comment has been minimized.

Copy link
@shishuo119

shishuo119 Aug 9, 2016

1.如果已知deleteList的类型为[]那么初始化时直接设为[],是不是更好?
2.对action的判断,我看了一下所有的代码,只有212行左右的checkOn函数用到了,并且action的参数是自己设置的,那么判断修改一下会不会更好?
引用checkOn函数的212行中的代码:

var action = isChecked ? 'add' : 'sub';
this.model.updateNum( action, skuId );

修改后的逻辑判断:

if ( action === 'add' ) {
……
}else{
……
}

This comment has been minimized.

Copy link
@Gavin-YYC

Gavin-YYC Aug 9, 2016

Author Contributor

你的意思是不用else if了是吧,我当时也是这么想的,但是转而一想,这else范围太大了,不行,我得控制一下,于是就加上了。。。纯属大脑活跃后做出的应激反应,写完就做写其他代码了~

This comment has been minimized.

Copy link
@shishuo119

shishuo119 Aug 9, 2016

结合使用场景,我觉得这个不用控制。。。

/**
* 批量删除收藏商品
*/
batchDelete: function () {
var deleteList = this.get('deleteList');
this.fetch({
url: '/collect/sku/del',
data: {sku_list: deleteList},

This comment has been minimized.

Copy link
@hongdou0216

hongdou0216 Aug 9, 2016

Contributor

直接赋值好还是先缓存好

datatype: 'json',
timeout: 20000,
success: function ( _, res ) {
if ( res && res.ret === 0 ) {
_.set('deleteStatus', res.content.status);
} else {
_.set('errMsg', that.ERROR_CODE[1]);
}
},
error: function ( _, err ) {
_.set('errMsg', that.ERROR_CODE[2]);
}
});
}
});

module.exports = Model;
Loading

0 comments on commit d00ecbd

Please sign in to comment.