Skip to content

Super branch#3

Open
GregHetherington wants to merge 2 commits intomasterfrom
superBranch
Open

Super branch#3
GregHetherington wants to merge 2 commits intomasterfrom
superBranch

Conversation

@GregHetherington
Copy link
Copy Markdown
Owner

So super!


// Configure the view for the selected state
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need this functions if you are not doing anything extra with them.

@property NSString * tweetText;
@property NSString * urlToImage;

-(id)initWithName:(NSString *)name_ andtweet:(NSString *)tweet_ andurl:(NSString *)url_;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice.

@property (weak, nonatomic) IBOutlet UITableView *tweetsTableView;
@property (weak, nonatomic) IBOutlet UIImageView *titleImage;
@property (weak, nonatomic) IBOutlet UIButton *searchButton;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are all these public?


- (BOOL)textFieldShouldReturn:(UITextField *)textField {
[textField resignFirstResponder];
if (textField == self.txtboxSearch) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't there only 1 textfield? You don't need to do this check if there is.


- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {

TweetsTableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:NSStringFromClass([TweetsTableViewCell class])];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NSStringFromClass is the way to go

AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithSessionConfiguration:configuration];
manager.requestSerializer = [AFJSONRequestSerializer serializer];

NSData *plainData = [[NSString stringWithFormat:@"%@:%@", @"CFMvDaHhOCwBeqMlPKDjeBODG", @"rV3L8VRTgCepmbVX25zXsSYPNzfczSv6kyky6AhwuLkr2ETYw1"] dataUsingEncoding:NSUTF8StringEncoding];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably don't want your authentication credentials in the app.

// TODO: add error log
}];
cell.username.text = self.users[[indexPath row]];
cell.tweet.text = self.tweets[[indexPath row]];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

row is a property. So do indexPath.row


// Change 10.0 to adjust the distance from bottom
if (maximumOffset - currentOffset <= 10.0) {
if(_pageAvailable)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can get rid of this nested If with (maxiumOffset - currentOffset <= 10.0 && _pageAvailable)

NSLog(@"Paging more data");
//static NSString * const BaseURLString = @"https://api.twitter.com/1.1/search/tweets.json";
//TODO: the fetch tweets call is using the base URL without the q=? parameter, you will have to modify the function for this call
//NSString *FinalURL = [BaseURLString stringByAppendingString:self.nextPageUrl];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out code

} failure:^(NSURLSessionTask *operation, NSError *error) {
NSLog(@"Error: %@", error);
}];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally you want to have a manager class that will make the network calls for you. However, this is ok for the project.

@JonathanYL JonathanYL removed their assignment Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants