Bug in TimelineComponent

TimelineComponent currently assumes that when it asks its target for a range of content, the union of its current range and the additional range is equal to the range represented by its contents.

This is not necessarily the case.

Consider the following scenario. As defined in the Makestagram tutorial, the timeline table view has a default range of [0, 5). If the user has, say, 3 posts, then after loading is complete, the timeline component's currentRange is [0, 5), but the contents array represents the range [0, 3). Suppose the user then uploads a series of posts, such that they now have 6 posts. They then scroll to the bottom of the list.

A bug manifests when we call targetWillDisplayEntry. It determines whether it should load more content by comparing the last index of the current range with the index of the entry being displayed. However, if the index of the last element in the contents array is less than the last index in the current range, no additional data will ever be loaded.

This could superficially be fixed by adding a condition such as || entryIndex == self.content.count - 1 inside targetWillDisplayEntry, which will cause the component to attempt to load more content if the entry being displayed is the last in the contents array despite the fact that its index doesn't match the last index in currentRange.

This could be fixed in a deeper way by more carefully maintaining the invariant that the length of the current range is always the same as the length of the contents array. After all, if both supposedly represent the same contiguous range of content, this is probably a desirable invariant.

[MakeSchoolBot]

Good catch :)
I'll take closer look at the potential fixes once I'm back at a computer!

On Wednesday, June 10, 2015, Warren Moore notifications@github.com wrote:

TimelineComponent currently assumes that when it asks its target for a
range of content, the union of its current range and the additional range
is equal to the range represented by its contents.

This is not necessarily the case.

Consider the following scenario. As defined in the Makestagram tutorial,
the timeline table view has a default range of [0, 5). If the user has,
say, 3 posts, then after loading is complete, the timeline component's
currentRange is [0, 5), but the contents array represents the range [0,
3). Suppose the user then uploads a series of posts, such that they now
have 6 posts. They then scroll to the bottom of the list.

A bug manifests when we call targetWillDisplayEntry. It determines
whether it should load more content by comparing the last index of the
current range with the index of the entry being displayed. However, if the
index of the last element in the contents array is less than the last
index in the current range, no additional data will ever be loaded.

This could superficially be fixed by adding a condition such as ||
entryIndex == self.content.count - 1 inside targetWillDisplayEntry, which
will cause the component to attempt to load more content if the entry being
displayed is the last in the contents array despite the fact that its
index doesn't match the last index in currentRange.

This could be fixed in a deeper way by more carefully maintaining the
invariant that the length of the current range is always the same as the
length of the contents array. After all, if both supposedly represent the
same contiguous range of content, this is probably a desirable invariant.

—
Reply to this email directly or view it on GitHub
#9.

---

I'm building the CS degree of the future @MakeSchool http://makeschool.com
Find me on twitter: @benjaminencz https://twitter.com/benjaminencz

[Ben-G]

OK, just looked into this. It turns out there isn't a trivial fix for the overall functionality. I have fixed the invariant, however we run into another problem.

The TimelineComponent and the ParseQueries are only set up to lazily load older posts. Let's assume a situation with the following four posts, the first is the newest:

A [In Range]
B [In Range]
C [In Range]
D [In Range]

Now we add a new Post, X. The new order of posts looks like this:

X [In Range]
A [In Range]
B [In Range]
C [In Range]
D [New]

The original range was 0..3. Now we have a range of 0..4. The TimelineComponent can now load 1 additional element (D) and will append it to the end. The rendered result looks like this:

A
B
C
D
D

The way the timeline is set up right now, a pull-to-refresh will load newer posts, while endless scrolling will only load older posts.

I don't think there's an easy fix that suites the scope of the project, but will be easiest do discuss in person.