Sorting an array of time-specific tasks of varying importance

RULES:

-
Strict tasks must occur at the time the user selected no matter what.

-
Flex tasks are in a priority order. Even if there is a gap big enough to fit several flex tasks with lower priority, no flex task can be scheduled until all the tasks with a higher priority are scheduled.

-
Flex tasks will continue to be rescheduled accordingly on refresh until a user completes the task.

-
Flex tasks can only be scheduled in gaps where the gap between strict tasks is bigger than the flex tasks duration.

I'm sharing this logic because I want to improve my coding style and approach. I'm aware there is probably lots of room for improvement.

Code:

```
-(NSMutableArray *)getAndSortAllTasks{

_strictTasksArray=[self getTasksInTimestampOrder];
_finalSortedArray=[_strictTasksArray mutableCopy];
_randomTasksArray=[self getRandomTasks];

NSLog(@"STRICT:%@",_strictTasksArray);
NSLog(@"RANDOM:%@",_randomTasksArray);

if(_randomTasksArray.count>0 && _strictTasksArray.count>0){

int strict_index=0;
int insert_index=0;
int flex_index=0;
int CurrentTStamp=[[NSDate date] timeIntervalSince1970];

int UPPER_LIMIT_TSTAMP,LOWER_LIMIT_TSTAMP,TIME_GAP;

/////////DEAL WITH GAP BETWEEN 'THE NOW' AND FIRST TASK.//////////

UPPER_LIMIT_TSTAMP=CurrentTStamp;
NSMutableDictionary *firstTask=[_strictTasksArray objectAtIndex:0];
NSLog(@"FIRST_TASK:%@",firstTask);
LOWER_LIMIT_TSTAMP= [[firstTask objectForKey:@"TASK_START_TIME"] intValue];
NSLog(@"LOWER_LIMIT_TSTAMP:%d",LOWER_LIMIT_TSTAMP);

if(_randomTasksArray.count>0){
for(flex_index=_randomTasksArray.count-1; flex_index >=0; flex_index--){

NSMutableDictionary *flexTask=[_randomTasksArray objectAtIndex:flex_index];
int FLEX_DURATION=[[flexTask objectForKey:@"TASK_DURRATION"] intValue];
int FLEX_START_TIME=[[flexTask objectForKey:@"TASK_START_TIME"] intValue];

I see many things that can be cleaned up here just to make the code clear and readable; I'm not sure I can be comprehensive with those, and because of that I'm going to mostly ignore the logic. I've tried to break the points down roughly by type-of-thing, and to go from easy-to-fix to more involved, although some items may straddle categories.
Consistency:

Sometimes operators (=, >=, +) in this code have spaces around them, sometimes not. Spaces are preferred style these days, but doing one or the other all the time in your code is the more important thing.

The asterisks in object variable declarations usually abut the variable name -- NSMutableDictionary firstTask -- but in one place they're stood off with a space -- NSSortDescriptor brandDescriptor. Here again, which style you use doesn't matter one iota (I personally like the latter, but most people seem to prefer the former) as long as it's the same throughout.

 

Likewise, a line with multiple declarations int UPPER_LIMIT_TSTAMP,LOWER_LIMIT_TSTAMP,TIME_GAP; is preceded by individual initializations like int flex_index=0;. Pick one style and be consistent.

 

Inter-line whitespace is haphazard: some blocks have an empty line at the top, some have none, at least one has two. Closing braces are not always lined up with their headers, and in one place have blank lines between them, in another not. It looks like you're trying to use whitespace inside blocks in a good way, to make groups out of statements that logically act together, but this could be more carefully considered in nearly every case.
Naming:

Don't name your methods get... anything. That has a special meaning in Cocoa; the method is expected to take a buffer of some kind to fill, and return information indirectly. None of the methods you've shown here fit the bill.

 

You say "Flex tasks are in a priority order". Why then is the variable holding them called "random"? That strongly implies that there's no order to the list. Call this simply flexTasks, or prioritizedFlexTasks if this is a sorted version of another collection.

 

Don't give variables capitalized names. In Objective-C, all-caps names are generally reserved for preprocessor definitions, things that never change. UPPER_LIMIT_TSTAMP, LOWER_LIMIT_TSTAMP, and TIME_GAP are all variables whose values change repeatedly. They should be called upper_limit_tstamp, lower_limit_tstamp, and time_gap.

 

The underscore in _strictTasksArray and the others implies that they are ivars. If that's so, consider whether it's necessary to set these ivars every time this method is called, or if they should already be set, and just accessed here. Also, if they're not being used elsewhere in the class, they should be locals, not ivars. Finally, if _finalSortedArray is an ivar, you most likely shouldn't be returning it from this method. Let access to it go through its getter method.

 

(Try not to name things ...Array. The arrays in this method would be fine named strictTasks, randomTasks, finalTasks, and sortedTasks. The plural clearly indicates that we've got a list.)
Types and framework methods:

timeIntervalSince1970 returns NSTimeInterval, not int. If you want to truncate (it's a double under the hood), use an explicit conversion.

 

All of the data access takes the form [[firstTask objectForKey:@"TASK_START_TIME"] intValue]. At first I thought you were storing quantities in strings; I later see that the dictionary values are NSNumbers, but still, consider replacing the dicts with a data class which holds the information you need, explicitly in the datatype you'll be using:

@interface CKLTask : NSObject 
@property (nonatomic) int startTime; 
@property (nonatomic) int duration; 
// etc. 
@end.

This makes your code more readable and lets your tools help you, by turning mistakes into compiler errors.

Alternatively, at least make named constants for your string keys. Notice that you've misspelled "duration" in @"TASK_DURRATION". If you were to spell it correctly elsewhere, or misspell this key in a different way, your access would fail (see the caution at the end for one possible consequence).

 

This loop:

for(flex_index=_randomTasksArray.count-1; flex_index >=0; flex_index--){

    NSMutableDictionary *flexTask=[_randomTasksArray objectAtIndex:flex_index];

might be better written with fast enumeration:

for (NSMutableDictionary *flexTask in [_randomTasksArray reverseObjectEnumerator])

(I know you're using that index inside the loop; see below about that.)

 

NSArray has a handy lastObject method that you can use instead of subtracting one from the count manually:

NSMutableDictionary *last_task=[_strictTasksArray objectAtIndex:_strictTasksArray.count-1];

Use NSArray literals and subscripting. [_strictTasksArray objectAtIndex:0]; is _strictTasksArray[0].

Here:

```
NSSortDescriptor * brandDescriptor = [[NSSortDescriptor alloc] initWi

StackExchange Code Review Q#122968, answer score: 2