shinygang / Vue-cnodejs

基于vue.js重写Cnodejs.org社区的webapp
http://shinygang.coding.me/
3.03k stars 803 forks source link

get topics page by page #78

Closed F-loat closed 7 years ago

shinygang commented 7 years ago

@F-loat 这个我们之前是这样处理的,但是在某些时间点会出现重复的情况,不建议这样弄。

F-loat commented 7 years ago

嗯,但是现在这种方式也不太合适,100条数据就有100+kb大小了,应该在判断是否有重复的帖子上做优化,另外可以增加下拉刷新之类的方式来更新最新的帖子

shinygang commented 7 years ago

@F-loat 你可以再加点代码,在列表渲染的时候根据id来作为key渲染,这样应该可以避免重复.

F-loat commented 7 years ago

我试了下,只是增加key好像并不能避免重复,key的作用应该只是避免再次渲染,应用场景是目前这种直接把整个数组重新赋值的,我加了一个temp对象,通过下面的方式来判断帖子是否已经存在,

if (d && d.data) {
    for (let topic of d.data) {
        if (!this.temp[topic.id]) {
            this.topics.push(topic);
            this.temp[topic.id] = true;
        }
    }
}
shinygang commented 7 years ago

@F-loat 存在的话用新的替换旧的吧。

F-loat commented 7 years ago

嗯,可以,我改改看

F-loat commented 7 years ago

额,这段代码应该可以实现,但是感觉太不优雅。。我待会儿再想想看有没有更好的办法

if (d && d.data) {
    d.data.forEach((topic, index) => {
        if (!this.temp[topic.id]) {
            this.topics.push(topic);
            this.temp[topic.id] = (this.searchKey.page - 1) * this.searchKey.limit + index + 1;
        } else {
            this.topics[this.temp[topic.id] - 1] = topic;
        }
    });
}
F-loat commented 7 years ago

现在看着舒服多了

shinygang commented 7 years ago

@F-loat

 mergeTopics(topic) {
 +                const isOld = this.topics.some((oldTopic, index) => {
 +                    if (topic.id === oldTopic.id) {
 +                        this.topics[index] = topic;
 +                        return true;
 +                    } else {
 +                        return false;
 +                    }
 +                });
 +                if (!isOld) this.topics.push(topic);
 +            }

这代理里面的isOld是不是有点多余,直接else里面this.topics.push(topic);就可以了。还有个问题就是这样判断重复的话效率不高,每次请求新数据来都的遍历。

F-loat commented 7 years ago

else里还没有遍历结束,但是这样写代码逻辑还是比较清楚的,而且没有产生额外的数据,可以再优化下,每次获取数据后只遍历一次

F-loat commented 7 years ago

暂时好像想不到更好的办法了,其实这样遍历也还好吧,目前这种每次都获取所有数据,之后vue不也要进行遍历,而且对比的量应该会更大

shinygang commented 7 years ago

@F-loat 目前这种方式,每次都先遍历最新数据,然后遍历老数据,相当于是2020page的遍历量,能不能吧之前的数据用对象的方式存储,然后每次来判断对象里面是否包含这数据,这样每次只遍历20次了。

F-loat commented 7 years ago

@shinygang 现在这样呢

shinygang commented 7 years ago

@F-loat

const topicsIndex = (this.searchKey.page - 1) * this.searchKey.limit + index + 1;
this.temp[topic.id] = topicsIndex;
 this.topics.push(topic);

这里是否可以改成:

this.topics.push(topic);
this.temp[topic.id] = this.topics.length - 1;
F-loat commented 7 years ago

@shinygang 有道理,这样是不是会更好些,另外我把temp改为index吧,语义更明确一些

if (d && d.data) {
    for (const topic of d.data) {
        if (typeof this.temp[topic.id] === 'number') {
            const topicsIndex = this.temp[topic.id];
            this.topics[topicsIndex] = topic;
        } else {
            this.temp[topic.id] = this.topics.length;
            this.topics.push(topic);
        }
    }
}