Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some null checks #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Some null checks #20

wants to merge 3 commits into from

Conversation

zaclimon
Copy link

Checks for possible NPE's when importing channels/programs.

zaclimon added 2 commits June 17, 2017 19:02
Some apps might not have the programs for a given channel. In
that case, they might return a null on getProgramsForChannel().

Verify that it the list of programs is correct before continuing.
.setChannelId(channelMap.valueAt(i).getId())
.build());

if (programs != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be cases when the program list will be null while using the library?

Copy link
Author

@zaclimon zaclimon Jun 23, 2017

Choose a reason for hiding this comment

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

In my case, the provider doesn't have all programs for a given channel in the EPG's XMLTV. Surely enough, I could simply pass an empty list instead of returning null on when getProgramsForChannel() is called.

@@ -344,7 +344,7 @@ private static Program parseProgram(XmlPullParser parser)
.setChannelId(channelId.hashCode())
.setTitle(title)
.setDescription(description)
.setPosterArtUri(icon.src)
.setPosterArtUri(icon != null ? icon.src : null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, will there be cases when the icon will be null using the library?

Copy link
Author

Choose a reason for hiding this comment

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

Same as above, since I can't control the the info put by the provider, some channels doesn't have an icon. This could be somehow bypassed by simply creating a "dummy" icon but it doesn't quite solve the problem.

@Fleker
Copy link
Contributor

Fleker commented Jun 25, 2017

I suppose a big question is around the philosophy of this library. To what extent should it assume properties exist and what should it try to handle?

If the former, then we should add @nonnull to those properties. If the latter, then your PR would be useful.

@zaclimon
Copy link
Author

I believe that both of the solutions proposed can be used for this PR. Here's my take on this:

  • If the user can play a role in the property's creation, then @NonNull can be added. (getProgramsForChannel() case)
  • However, if the user can't really do anything since it is handled entirely by the library, then it should be fixed from within. (setPosterArtUri() case)

Either solutions can be interchangeable but it depends on the level of control that the user have over the library. The open source variant can be changed easily to tailor the required needs. However, the packaged one could be a bit harder to play with.

Since it might not be possible to control the content coming from
an XMLTV provider, if a program isn't valid after being parsed,
simply skip it.

Notify the user about the faulty program in case.
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