Merge lp:~tintou/appcenter/optimizations into lp:~ricotz/appcenter/optimizations

Proposed by Corentin Noël
Status: Needs review
Proposed branch: lp:~tintou/appcenter/optimizations
Merge into: lp:~ricotz/appcenter/optimizations
Diff against target: 36 lines (+8/-9)
1 file modified
src/Core/Package.vala (+8/-9)
To merge this branch: bzr merge lp:~tintou/appcenter/optimizations
Reviewer Review Type Date Requested Status
Rico Tzschichholz Pending
Review via email: mp+309464@code.launchpad.net

Description of the change

I think it's better like this, tell me if it's a bad idea

To post a comment you must log in.
Revision history for this message
Rico Tzschichholz (ricotz) wrote :

The boolean variable is not meant to catch race-conditions with threads, but simply the case if there is no matching package found or an error occurred, which the API suggests could happen.

If this function gets actually called in a multi-threaded environment adding a lock would make sense though.

Revision history for this message
Zisu Andrei (matzipan) wrote :

Rejecting as per Rico's comment.

Revision history for this message
Zisu Andrei (matzipan) wrote :

Whoops. Appears I can't do that myself. Rico?

Unmerged revisions

331. By Corentin Noël

Use existing variables.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/Core/Package.vala'
--- src/Core/Package.vala 2016-10-26 14:24:07 +0000
+++ src/Core/Package.vala 2016-10-27 12:59:03 +0000
@@ -40,7 +40,6 @@
40 public State state { public get; private set; default = State.NOT_INSTALLED; }40 public State state { public get; private set; default = State.NOT_INSTALLED; }
4141
42 private Pk.Package? package = null;42 private Pk.Package? package = null;
43 private bool package_retrieved = false;
4443
45 public double progress {44 public double progress {
46 get {45 get {
@@ -289,16 +288,16 @@
289 return null;288 return null;
290 }289 }
291290
292 if (package_retrieved)291 lock (package) {
293 return package;292 if (package == null) {
294293 try {
295 try {294 package = AppCenterCore.Client.get_default ().get_app_package (component.get_pkgnames ()[0], 0LL);
296 package = AppCenterCore.Client.get_default ().get_app_package (component.get_pkgnames ()[0], 0LL);295 } catch (Error e) {
297 } catch (Error e) {296 critical (e.message);
298 critical (e.message);297 }
298 }
299 }299 }
300300
301 package_retrieved = true;
302 return package;301 return package;
303 }302 }
304}303}

Subscribers

People subscribed via source and target branches