PureDark / H-Viewer

An android feed reader application which fetch data with selector and regular expression.
Apache License 2.0
1.74k stars 236 forks source link

优化导入已下载 #45

Closed gedoor closed 7 years ago

gedoor commented 7 years ago

倒入下载重新获取路径时间很长,

On Mon, Dec 26, 2016 at 7:26 PM +0800, "PureDark" notifications@github.com<mailto:notifications@github.com> wrote:

@PureDark requested changes on this pull request.

总体来说这次的改动并没有真正改进下载,进行的变动增加了冗余,不予通过


In app/src/main/java/ml/puredark/hviewer/dataholders/DownloadTaskHolder.javahttps://github.com/PureDark/H-Viewer/pull/45#pullrequestreview-14393466:

@@ -112,23 +113,35 @@ public int scanPathForDownloadTask(String rootPath, String... subDirs){ DownloadTask task = new Gson().fromJson(detail, DownloadTask.class); task.status = DownloadTask.STATUS_COMPLETED; LocalCollection collection = task.collection;

  • DocumentFile ifile;

filename只是一个临时变量,用来确保导入的时候把对应图片的 URI更新为文件URI,除此之外别无它用,专门存起来没有任何意义啊,因为这个信息本来就是包含在cover属性中的,filename只用来临时存从路径中分离出来的文件名信息


In app/src/main/java/ml/puredark/hviewer/dataholders/DownloadTaskHolder.javahttps://github.com/PureDark/H-Viewer/pull/45#pullrequestreview-14393466:

                     if (ifile != null) collection.cover = ifile.getUri().toString();

for (Picture picture : collection.pictures) {

  • filename = picture.thumbnail.substring(picture.thumbnail.lastIndexOf("%2F")+3,picture.thumbnail.length());
  • ifile = dir.findFile(filename);
  • if (picture.filename == null) {
  • picture.filename = picture.thumbnail.substring(picture.thumbnail.lastIndexOf("%2F")+3,picture.thumbnail.length());
  • if (picture.filename.contains("/")) {
  • picture.filename = picture.filename.substring(picture.filename.lastIndexOf("/") + 1, picture.filename.length());
  • }
  • }
  • ifile = dir.findFile(picture.filename);

picture的也一样,这两个地方的改动都没有作用,并增加了一个用不上的属性,文件名信息本就包括在路径中,另外多一个属性首先是没必要,其次是如果两个属性中的文件名信息不一致以哪个为准


In app/src/main/java/ml/puredark/hviewer/download/DownloadService.javahttps://github.com/PureDark/H-Viewer/pull/45#pullrequestreview-14393466:

  • }
  • Handler handler= new Handler(){
  • public void handleMessage(Message msg) {
  • if (msg.what == -1) {
  • mBuilder.setContentText("导入失败");
  • } else if (msg.what == 0) {
  • mBuilder.setContentText("导入完成-未发现已下载图册");
  • } else {
  • mBuilder.setContentText("导入成功" + msg.what + "个已下载图册");
  • }
  • run = false;
  • mBuilder.setOngoing(false);
  • mNotificationManager.notify(mid,mBuilder.build());
  • }
  • };

仅仅是导入的过程为什么会用到通知栏,这不符合使用情景吧,导入的过程一般持续最多几秒钟,用户本来就期望点了之后很快就弹出提示框提示结果,而就为了一次导入而向本来就拥挤的通知栏塞入一条消息?目前我觉得唯一值得使用通知栏的只有当前下载进度


In app/src/main/java/ml/puredark/hviewer/ui/fragments/SettingFragment.javahttps://github.com/PureDark/H-Viewer/pull/45#pullrequestreview-14393466:

@@ -280,7 +281,7 @@ public boolean onPreferenceTreeClick(PreferenceScreen preferenceScreen, Preferen new AlertDialog.Builder(activity).setTitle("确定要导入已下载图册?") .setMessage("将从当前指定的下载目录进行搜索") .setPositiveButton(getString(R.string.ok), (dialog, which) -> {

  • DownloadedImport();
  • new DownloadService().DownloadedImport();

导入是DownloadTaskHolder管的事情,Service只负责在后台下载,将DownloadedImport整合进Service,在调用时会不知道调用的Context,最后就像你这样只能用通知栏,但是这里的导入下载,明显是设置界面的功能,而不是一个需要后台调用的功能,作为一个界面点击操作的反馈,就应该在当前界面反馈结果

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/PureDark/H-Viewer/pull/45#pullrequestreview-14393466, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVpm77O_lfh5vbu4GHBa3Tha8ZDf9GOwks5rL6RzgaJpZM4LVf9X.

PureDark commented 7 years ago

那么应该加一个Dialog,放Loading转圈、导入的操作肯定是不能放后台,不能让用户点了就可以退出去看其他的,让用户等待导入完毕,不然容易出纰漏

wspl commented 7 years ago

所以为了避免已提交代码的浪费,在进行交互方式的修改(非添加)之前请先提个 issues,讨论一下可行性,若大部分 contributor 觉得合理,再推送 pr。