Apply suggestions from code review

Co-authored-by: Joel Fischer <[email protected]>
smartdevicelink · Jun 3, 2021 · 60061c6 · 60061c6
1 parent 9a51e8f
commit 60061c6
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 8 deletions.
diff --git a/SmartDeviceLink/private/SDLChoiceSetManager.m b/SmartDeviceLink/private/SDLChoiceSetManager.m
@@ -287,10 +287,11 @@ - (void)preloadChoices:(NSArray<SDLChoiceCell *> *)choices withCompletionHandler
                 [strongSelf.preloadedMutableChoices unionSet:choicesToUpload.set];
                 [strongSelf.pendingMutablePreloadChoices minusSet:choicesToUpload.set];
             } else {
-                NSMutableSet *successfulChoiceUploads = [NSMutableSet setWithSet:choicesToUpload.set];
+                // If some choices failed, remove the failed choices from the successful ones, then update the preloaded choices and pending choices
+                NSMutableSet<SDLChoiceCell *> *successfulChoiceUploads = [NSMutableSet setWithSet:choicesToUpload.set];
                 [successfulChoiceUploads minusSet:failedChoiceUploadSet];
 
-                [strongSelf.preloadedMutableChoices unionSet:[successfulChoiceUploads copy]];
+                [strongSelf.preloadedMutableChoices unionSet:successfulChoiceUploads];
                 [strongSelf.pendingMutablePreloadChoices minusSet:choicesToUpload.set];
             }
         }];
@@ -398,7 +399,7 @@ - (void)presentChoiceSet:(SDLChoiceSet *)choiceSet mode:(SDLInteractionMode)mode
     [self preloadChoices:self.pendingPresentationSet.choices withCompletionHandler:^(NSError * _Nullable error) {
         __strong typeof(weakSelf) strongSelf = weakSelf;
         if (error != nil) {
-            SDLLogE(@"Error preloading choice cells. Aborting presentation.");
+            SDLLogE(@"Error preloading choice cells for choice set presentation; aborting. Error: %@", error);
             [choiceSet.delegate choiceSet:choiceSet didReceiveError:error];
             return;
         }
@@ -408,6 +409,7 @@ - (void)presentChoiceSet:(SDLChoiceSet *)choiceSet mode:(SDLInteractionMode)mode
             return;
         }
 
+        // The cells necessary for this presentation are now preloaded, so we will enqueue a presentation
         [strongSelf sdl_presentChoiceSetWithMode:mode keyboardDelegate:delegate];
     }];
 }

diff --git a/SmartDeviceLink/private/SDLPreloadChoicesOperation.m b/SmartDeviceLink/private/SDLPreloadChoicesOperation.m
@@ -41,7 +41,7 @@ @interface SDLPreloadChoicesOperation()
 @property (weak, nonatomic) id<SDLConnectionManagerType> connectionManager;
 @property (weak, nonatomic) SDLFileManager *fileManager;
 @property (copy, nonatomic, nullable) NSError *internalError;
-@property (strong, nonatomic, nullable) NSMutableArray<NSNumber *> *failedChoiceUploadIDs;
+@property (strong, nonatomic) NSMutableArray<NSNumber *> *failedChoiceUploadIDs;
 
 @end
 

diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLChoiceSetManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLChoiceSetManagerSpec.m
@@ -626,7 +626,7 @@ - (void)sdl_addUniqueNamesToCells:(NSMutableSet<SDLChoiceCell *> *)choices;
                 });
             });
 
-            it(@"It should skip preloading the choices if all choice items have already been uploaded", ^{
+            it(@"should skip preloading the choices if all choice items have already been uploaded", ^{
                 OCMExpect([strickMockOperationQueue addOperation:[OCMArg checkWithBlock:^BOOL(id value) {
                     SDLPreloadChoicesOperation *preloadChoicesOperation = (SDLPreloadChoicesOperation *)value;
                     expect(testManager.pendingMutablePreloadChoices.count).to(equal(3));
@@ -677,7 +677,7 @@ - (void)sdl_addUniqueNamesToCells:(NSMutableSet<SDLChoiceCell *> *)choices;
                 OCMVerifyAllWithDelay(choiceDelegate, 0.5);
             });
 
-            it(@"It should upload choices that failed to upload in previous presentations", ^{
+            it(@"should upload choices that failed to upload in previous presentations", ^{
                 NSMutableDictionary<SDLRPCRequest *, NSError *> *testErrors = [NSMutableDictionary dictionary];
                 SDLCreateInteractionChoiceSet *failedChoiceSet = [[SDLCreateInteractionChoiceSet alloc] initWithId:0 choiceSet:@[[[SDLChoice alloc] initWithId:1 menuName:@"1" vrCommands:nil]]];
                 testErrors[failedChoiceSet] = [NSError sdl_choiceSetManager_choiceUploadFailed:[NSDictionary dictionary]];

diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m
@@ -320,7 +320,7 @@ @interface SDLChoiceCell()
                 testCells = [[NSOrderedSet alloc] initWithArray:@[testCell1, testCell2]];
             });
 
-            context(@"if a choice item is removed", ^{
+            describe(@"if a choice item is removed", ^{
                 it(@"should be removed if the removal is attempted while the operation is pending", ^{
                     SDLTextField *primaryTextField = [[SDLTextField alloc] init];
                     primaryTextField.name = SDLTextFieldNameMenuName;
@@ -354,7 +354,7 @@ @interface SDLChoiceCell()
             });
         });
 
-        context(@"the module's response to choice uploads", ^{
+        describe(@"the module's response to choice uploads", ^{
             __block SDLChoiceCell *testCell1 = nil;
             __block SDLChoiceCell *testCell2 = nil;
             __block NSOrderedSet<SDLChoiceCell *> *testCells = nil;