Advertise here




Advertise here

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

UITableViewCell repeat every 12 rows

peejaverypeejavery Posts: 211Registered Users @ @
edited October 2009 in iPhone SDK Development
Using the following code, when I click a cell to create a checkmark accessory, it repeats the checkmark every 12 rows. Any ideas as to why?
- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
	UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];
	if (cell.accessoryType == UITableViewCellAccessoryNone) {
		cell.accessoryType = UITableViewCellAccessoryCheckmark;
	}
	else if (cell.accessoryType == UITableViewCellAccessoryCheckmark) {
		cell.accessoryType = UITableViewCellAccessoryNone;
	}
    [tableView deselectRowAtIndexPath:indexPath animated:NO];
}
Post edited by peejavery on
«1

Replies

  • RickMaddyRickMaddy Posts: 2,122New Users
    edited December 2008
    The code looks correct. Is it possible that 'cellForRowAtIndexPath:' is returning the wrong cell at times?

    Perhaps the logic for setting the accessory type in 'cellForRowAtIndexPath:' isn't updated properly.

    If you are allowing any number of rows to be checked then you need a separate data model to track which rows are checked and which aren't so you can set the accessory type properly in 'cellForRowAtIndexPath:'.

    Remember, cells get reused so you can't rely on the current value of the accessory type as an indicator of whether is was checked or not.
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    You should implement:
    - (UITableViewCellAccessoryType)tableView:(UITableView *)tableView accessoryTypeForRowWithIndexPath:(NSIndexPath *)indexPath
    
  • RickMaddyRickMaddy Posts: 2,122New Users
    edited December 2008
    But you still need to change the accessory type in 'didSelectRow' if you want to toggle checkmarks.
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    That isn't sufficient. In fact if you update your model appropriately you could just reloadData as long as you've implemented accessoryTypeForRowWithIndexPath:

    It might be possible to not implement accessoryTypeForRowWithIndexPath if you set the accessory correctly in cellForRowAtIndexPath, but you need to do it elsewhere than just in didSelectRowAtIndexPath: otherwise the checkmark will be lost when the cell scrolls off and back on and may be turned on incorrectly for random rows, like the OP reports.

    Oh, and just for fun, you can toggle between the two values like this
    cell.accessoryType = (UITableViewCellAccessoryNone + UITableViewCellAccessoryCheckmark) - cell.accessoryType;
    
  • RickMaddyRickMaddy Posts: 2,122New Users
    edited December 2008
    I knew what you meant. I was just saying that in addition to coding 'accessoryTypeForRow...' you still need to toggle the accessory in 'didSelectRow...'. I just wasn't clear enough.

    OK, now I see what you are saying. Call 'reloadData' instead of toggling in 'didSelectRow'. That seems pretty inefficient just to toggle a checkmark.

    Regardless, all approaches require the data model tracking the checked state to be updated properly.
  • peejaverypeejavery Posts: 211Registered Users @ @
    edited December 2008
    You should implement:
    - (UITableViewCellAccessoryType)tableView:(UITableView *)tableView accessoryTypeForRowWithIndexPath:(NSIndexPath *)indexPath
    
    Okay, I have implemented this, and the duplicates have disappeared. However, everytime a checkmark scrolls off the screen, it too disappears. The moment I scroll back, the checkmark does not appear.

    Also, is there a way to do checkmarks to the left and not the right?
  • RickMaddyRickMaddy Posts: 2,122New Users
    edited December 2008
    Show how you implemented the 'accessoryView...' method. Are you setting the accessory type based on your data model? Are you updating the data model in the 'didSelectRow...' method?
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    peejavery,

    What I think you are missing is something that eluded me at first.

    cellForRowAtIndexPath just sets up each specific cell at an instant in time-- when it is about to be displayed. A cell will be redrawn any time the cell scrolls* into view. Cells that never scroll* into view are never set up.

    *or is one of the cells that fits into the initial view of the table.

    The cell is an empty container that will be completely regenerated each time it is about to be displayed.

    So anything you put in a cell, must be stored externally (from the cell) so that cellForRowAtIndexPath can include it whenever it needs to redraw the cell.

    accessoryForRowAtIndexPath is just a specialized subset of cellForRowAtIndexPath.

    The best way to visualize this is in cellForRowAtIndexPath:

    1) put a dynamic variable, say current time H:M:S or an incremented external counter in each cell.

    2) NSLog the cell and dynamic variable

    Now, scroll the cells off and on the screen and watch the log.


    HTH

    Dick

    P.S.

    Now that I think of it, this is an excellent example of MCV in use:

    -- The View is the cell that is to be drawn
    -- The Controller is cellForRowAtIndexPath (within the TableViewController)
    -- The Model is the cell's data (title, checkmark, etc,) that is stored external to the View or Controller.
  • HenningHenning Posts: 168Registered Users
    edited December 2008
    An easy but expensive way to get around these problems is to use a different Cell Identifier for each cel. That way, cels don't get reused and you always have unique information in each.

    However, this is expensive because a cell has to be created for each row and won't get reused.
    from <a href="http://www.chewyapps.com/" target="_blank">ChewyApps.com</a>
  • peejaverypeejavery Posts: 211Registered Users @ @
    edited December 2008
    Thanks a lot everyone! dicklacara, that really helped me to understand the issue!
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    john wrote: »
    An easy but expensive way to get around these problems is to use a different Cell Identifier for each cel. That way, cels don't get reused and you always have unique information in each.

    However, this is expensive because a cell has to be created for each row and won't get reused.

    I am no expert, but wouldn't that be "fighting the system"?

    If you look into the Cocoa philosophy/structure behind tables it is a rather elegant package.

    For example, if we were displaying a large table on a web page (one that would not entirely fit on the display) we would just pump out the html to format and draw every row and column, then let the browser's scroll bars do the work of scrolling to the row/column we want to view. This is expensive in bandwidth, time, and CPU cycles... beside being a crappy UI, it could be a real snoozer while waiting for the table to load and display.

    Not to mention what you would need to do if you wanted to resize/add/move/delete a row or column... or just sort by columns.

    Cocoa provides a rather elegant alternative with NSTableView (the big brother to UITableView). instead of scrolling the window by the data as above, we scroll the data by the window (view). The advantages are:

    1) we can draw a window (view) that entirely fits on the screen, can be resized, and displays a subset of the rows and columns.

    2) we use the view's scrollbars to bring other rows/columns into view

    3) We only need to format (or manipulate) those rows and columns that are currently displayed, for example: we have a table consisting of 100 rows and 20 columns; we format and display a subset of 10 rows of 15 columns in our table view.

    In fact, we don't really format the individual cells, we just tell the view controller (delegate) what each cell looks like and where to get the data (datasource). It figures out which cells need to be formatted, with what, and it displays them.

    Now we want to add (or delete or resize) say, the 3rd row (of whichever rows are currently displayed). All we need to deal with is the one row we want to change. If we delete a row, the view controller detects this and realizes that it has some work to do: remove row 3; move rows 4-10 up; and add an new 10th row (previously not displayed) to the bottom.

    UITableView does all this (only with a table with a single column).

    My point is: if you let UITableView and friends do their thing, you can just sit back and enjoy the show-- knowing that the work is being done precisely and efficiently (behind the scene).

    Have a look at Table View Programming Guide in the docs.

    HTH

    Dick
  • HenningHenning Posts: 168Registered Users
    edited December 2008
    Uh, I'm not sure you understood my post. Even if you use a different cell identifier for each cell, you're still using the table view and are still getting all the benefits thereof.
    from <a href="http://www.chewyapps.com/" target="_blank">ChewyApps.com</a>
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    john wrote: »
    Uh, I'm not sure you understood my post. Even if you use a different cell identifier for each cell, you're still using the table view and are still getting all the benefits thereof.
    Sorry, my bad!

    Dick
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    peejavery wrote: »
    Thanks a lot everyone! dicklacara, that really helped me to understand the issue!

    And have you solved the problem?

    @Dick, yes OP has misunderstood the basic functioning of UITableView and how it exemplifies MVC design. There always must be an independent data model whose state is complete and correct. The tableview will only manipulate/display those cells that are visible on screen. The various callbacks must configure the cells correctly every time. Dynamic state, like a checkmark, must be stored in the data model and must be set, both checked or unchecked, for the cells when created or in the callback that exists specifically to set accessory state.

    @Rick, As we agreed, the checkmark state must be set in two places. You are mistaken that reloadData has a lot of overhead, although in this case as long as the data model is updated it's perfectly OK to set the checkmark state when the cell is selected, and not reloadData.

    @John, It's a bad suggestion to recommend that cells not be reused. Especially since it's an attempt to paper over the OP's misunderstanding of how UITableView works. Not reusing cells can result in out-of-memory errors and will result in jerky scrolling for tables that are very big, say more than ten rows.
  • RickMaddyRickMaddy Posts: 2,122New Users
    edited December 2008
    ... You are mistaken that reloadData has a lot of overhead...

    We may be picking nits here but I'm curious about this statement. It's a relative thing. Compared to just toggling the accessory view in the 'didSelectRow...', calling 'reloadData' IS a lot of extra overhead.

    Calling reloadData is going to result in the table calling:

    numberOfSections...
    numberOfRowsInSection...
    cellForRow...

    plus any others it may need to completely create and redraw the visible cells.

    I'd call that a lot of overhead.

    We agree on functionality, we are just debating finer details and semantics :)
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    @phoney, @rick

    Isn't this one of those murky areas between doing what is PC (Programmatically Correct) from an OO/MVC perspective and what is accepted practice (if you know what you are doing) from an efficiency perspective.

    From a PC perspective you should update the data (model) and tell the controller to do its thing to regenerate the view.

    From an efficiency perspective, just update the data, update the cell (view), and get on with it!

    The PC technique handles any condition that could arise (say the update to the cell changes the size of the cell).

    The Efficiency technique gets the job done with as little overhead as possible-- fairly important on a battery device (those cycles do add up).

    The big trap, IMO, is that when one looks at the code for the Efficiency technique, one tends to think that this is "the correct way" to do things.

    If it were just setting a standard accessory, I would opt for the Efficiency technique... with WITH a comment in the code to explain what I was doing and why!

    Dick
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    @Dick, both are correct. I don't see any philosophical differences between the two.

    @Rick, If you read my original comment I was trying to make the point that one COULD set the data model and then call reloadData, if everything was implemented correctly. I wasn't really saying that one SHOULD.

    To make this more clear, because of the MVC design of UITableView all of the contents of the tableview must be maintained separately in a data model. For static content, like labels, this content is set once and never changes. For dynamic content, like the state of a control in the table or a timer or especially a textfield, any changes in the control's state must be stored in the data model. For a timer, changes in the time must be sent back to the rows when the time changes. The textfield issue has come up many times in this forum. Conceptually, changes in the data model are sent back to the rows by calling reloadData.

    So in the case at hand, if the coder has set up both his data model and the code that sets the content of the rows correctly, updating the data model and then calling reloadData should result in the correct appearance of the rows. It isn't required to explicitly set the state of the checkmark when the row is selected.

    There are also some benefits of doing it that way. There will only be one place where the checkmark is set, which contributes to simplicity. Simplicity contributes to correctness.

    Efficiency is a separate issue. I don't deny that setting the checkmark directly will use less code than calling reloadData. I do deny that you know how much less code will be executed. I believe that in most cases the difference is unimportant and indistinguishable to the user holding the device. In my experience it's very fast. Remember that reloadData will only reload the visible cells, usually about ten.

    In the case at hand setting the checkmark is a simple thing so I would probably do it directly in my code also. However, in plenty of cases resetting a cell or updating a single control for all cells is complicated and mostly duplicates the code in cellForRowAtIndexPath. It can be difficult or impossible to know which cells are really visible. In those cases just update your data model, call reloadData, and don't feel guilty about it. The deciding factor would be complexity of the code more than anything else. Only if the simpler solution was clearly slow would I spend a lot of effort improving a complicated case.
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    Thanks, guys!

    This is a real help to me... someone who is still learning, but starting to get my mind around how things are "supposed to be done" and when/why to deviate from the "supposed to be done" way.

    One thing I am learning is: to look at examples, suggestions and advice with a little skepticism....

    Sort of: Here is an SDK example, snippet of code, suggested technique:

    1) It is just one way of doing things
    2) Why is he doing it that way?
    3) Is there a better way? Why?

    Many of the earlier SDK examples no longer reflect how things should be done (especially with the new app templates and IB improvements).

    There have been improvements to Xcode and the SDK that obviate earlier examples.

    Some of the experts are just those who are on page 5 while I am still on page 2 (not to demean them, but this is all new and we are all in various stages of learning).

    Dick
  • peejaverypeejavery Posts: 211Registered Users @ @
    edited December 2008
    And have you solved the problem?
    I did, but now I am having the same problem of repeats with a subclassed UITextField.

    The following code makes the UITextField reappear every 12 cells even though the log only shows "0-0" when the very first cell is in view.
    NSString *group = [NSString stringWithFormat:@"%d-%d", indexPath.section, indexPath.row];
        NSLog(group);
        if ([group isEqualTo:@"0-0"]) { // even tried matching section and row of the indexPath
            // subclass a textfield for the first tableview cell
            UITextField *textField = [[UITextField alloc] initWithFrame:CGRectMake(3, 3, cell.frame.size.width - 26, cell.frame.size.height - 7)];
            textField.autocorrectionType = UITextAutocorrectionTypeNo;
            textField.borderStyle = UITextBorderStyleRoundedRect;
            [cell.contentView addSubview:textField];
            [textField release];
        }
        else {
            [cell setText:@"Testing..."];
        }
    
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    @Dick, I'm coming to the conclusion that there are no experts. Not even at Apple. There's definitely a lot of misinformation and out-of-date information.

    @peejavery, Ah hah, UITextField in a UITableViewCell rears its ugly head.

    (Use isEqualToString: to compare two strings.)

    First, you need to do what I said. Store the text from the textfield in your data model whenever the text changes. You need one of the textfield delegate methods to do this.

    Second, restore this text in cellForRowAtIndexPath.

    Do you understand the significance of the number 12 that you are seeing?
  • HenningHenning Posts: 168Registered Users
    edited December 2008
    @John, It's a bad suggestion to recommend that cells not be reused. Especially since it's an attempt to paper over the OP's misunderstanding of how UITableView works. Not reusing cells can result in out-of-memory errors and will result in jerky scrolling for tables that are very big, say more than ten rows.

    But it's still an option that coders, not knowing the price of doing it, might try. I should have been more clear that this option is TOO expensive.
    from <a href="http://www.chewyapps.com/" target="_blank">ChewyApps.com</a>
  • peejaverypeejavery Posts: 211Registered Users @ @
    edited December 2008
    Second, restore this text in cellForRowAtIndexPath.
    That is exactly where my code is. I don't understand it. Here is the full snippet. The if statement still fires every 12 cells even though it doesn't meet the criteria.
    - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
        static NSString *CellIdentifier = @"Cell";
        UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];
        if (cell == nil) {
            cell = [[[UITableViewCell alloc] initWithFrame:CGRectZero reuseIdentifier:CellIdentifier] autorelease];
        }
    
        NSString *group = [NSString stringWithFormat:@"%d-%d", indexPath.section, indexPath.row];
        NSLog(group);
        if (indexPath.section == 0 && indexPath.row == 0) {
            // subclass a textfield for the first tableview cell
            UITextField *textField = [[UITextField alloc] initWithFrame:CGRectMake(3, 3, cell.frame.size.width - 26, cell.frame.size.height - 7)];
            textField.autocorrectionType = UITextAutocorrectionTypeNo;
            textField.borderStyle = UITextBorderStyleRoundedRect;
            [cell.contentView addSubview:textField];
            [textField release];
        }
        else {
            [cell setText:@"My text goes here"];
        }
    
        return cell;
    }
    
    Do you understand the significance of the number 12 that you are seeing?
    Yes, it's how many cells show on the screen at a time. It has to do with cell recycling. That was already explained by Dick.
  • PhoneyDeveloperPhoneyDeveloper Posts: 1,431Registered Users
    edited December 2008
    You have two different kinds of cells but you're not setting the cell identifier to distinguish between them. When the cells are reused some of them will already have the textfield added to them, some will just have the text specified.

    If you want to have a single row that's different from all the others that's fine. You need to set a different cell id and check for it. Something like this:
    - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
        static NSString *CellIdentifier = @"Cell";
        UITableViewCell *cell;
    
        if (indexPath.section == 0 && indexPath.row == 0) {
    
        cell = [tableView dequeueReusableCellWithIdentifier:TextFieldCellIdentifier];
        if (cell == nil) {
            cell = [[[UITableViewCell alloc] initWithFrame:CGRectZero reuseIdentifier:TextFieldCellIdentifier] autorelease];
         // subclass a textfield for the first tableview cell
            UITextField *textField = [[UITextField alloc] initWithFrame:CGRectMake(3, 3, cell.frame.size.width - 26, cell.frame.size.height - 7)];
            textField.autocorrectionType = UITextAutocorrectionTypeNo;
            textField.borderStyle = UITextBorderStyleRoundedRect;
            [cell.contentView addSubview:textField];
            [textField release];
         }
         // you need to set the text for the textField here 
    
        } else {
        cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];
    
        if (cell == nil) {
            cell = [[[UITableViewCell alloc] initWithFrame:CGRectZero reuseIdentifier:CellIdentifier] autorelease];
        }
    
             [cell setText:@"My text goes here"];
        }
    
        return cell;
    }
    
  • peejaverypeejavery Posts: 211Registered Users @ @
    edited December 2008
  • dicklacaradicklacara Posts: 123Registered Users
    edited December 2008
    ...Do you understand the significance of the number 12 that you are seeing?

    What an answer!

    Wish I'd thought of that.


    Ya' know, this whole thread has got my wheels turning. When I first learned to program the IBM 650 (1958) there was no CS, no OO-- hell, there weren't any compilers. There was this thing called SOAP (Symbolic Optimal Assembly Program) that would take single-address instructions (written symbolically) and assemble them into a working program that would be deployed on a magnetic drum in such a way to minimize latency (when you were ready to access an instruction (or data) it would appear under the drum read/write head so it could be pulled into working memory (cathode ray tubes, AIR) to be executed (upon).

    There were quite a few people who didn't trust these newfangled assemblers... real men coded in binary (or maybe octal).

    But, in those days, you learned to program from the ground up-- learning basic concepts, then built on those over time.

    Somewhere, at least to my eyes, that process has been lost or glossed over in learning to program the iPhone.

    Most tutorials start with some sort of single view/window showing "Hello World". Without much ado, we add a button and an action to modify a Label... but we never did learn what a view/window is!

    Next we jump into a TableView and all this stuff about delegates, datasources, yadda, yadda, yadda.

    Pretty soon we are writing apps (such as they are) that use pretty sophisticated UIs, frameworks, and objects... But we never understand what they/we are doing for/to each other.

    No foundation has been laid!

    A fellow programmer expressed it best: "I don't understand all I know about it!"

    What if there were a tutorial (a series, actually) that took the student through the process of manually building a table from basic objects and views. Then one-by-one, developed the need for scrolling, cell reuse, a datasource, call backs, a delegate...

    Then after doing it the "hard way" gradually open the Kimono to show the student the "easy way" with UITableView and friends.

    Say, start with a simple table with 5 rows... then increase the number of rows to n+1 (where n is the number of rows that can be displayed at one time). That +1 changes everything... the way we think about our table, the complexity... the new things we need to take care of.

    I bet the student would understand, appreciate and be able to problem-solve table issues!

    Prolly with less lines of code than has been expended in this thread!

    I know I would like to learn from such a tutorial!

    Thoughts?


    Dick
«1
Sign In or Register to comment.