Discussion: Inconsistency in Status entity Boolean methods and hard-to-access Visibility constants

[MastodonContentMover]

Am opening this issue after discussion with @bocops who sees the following two issues as both related to Java interoperability of the Status entity, and has suggested logging them together as one issue.

Issue 1: Inconsistency in methods returning Boolean values

The Status entity methods isFavourited(), isBookmarked() and isPinned() are all declared within BigBone as returning Boolean objects (instances of the Boolean class, rather than boolean primitive values). The Mastodon API also refers to the related API methods as returning Boolean objects.

However, when used from Java, isFavourited() and isBookmarked() always return primitive boolean values.

isPinned() also returns a primitive boolean value, except when the Status is not pinnable (a direct message, for example) — then it returns a Boolean object that is a null reference. In Java a primitive is never null.

To code defensively, it's necessary to check for null values, but both isFavourited() and isBookmarked() fail when this is attempted because the compiler recognizes them as always returning primitives. Here's the code I've ended up with:

           newPost.setIsFavourited(s.isFavourited());
           newPost.setIsBookmarked(s.isBookmarked());
           if (s.isPinned() != null) {   newPost.setIsPinned(s.isPinned());   }

The need to approach the methods in an inconsistent way is likely to be disorienting, even for those referring to the BigBone code (as a Java programmer, it appears to suggest the methods all return Booleans). That isPinned() sometimes returns a primitive and sometimes returns a (null) object also seems quite alien from a Java perspective.

Issue 2: Setting Visibility when posting a new status based on data from an existing Status object

The getVisibility() method on the Status object returns a String object, but the postStatus(...) method that calls the statuses API requires a Status.Visibility object.

In practice, this makes saving the data from one Status object and then trying to repost it not trivial; it is necessary to test the String value obtained from the first Status object against each of the four possible Status.Visibility constants in order to get a reference to the correct Status.Visibility object.

In closing

I've been able to work around both of these issues, but am noting them here as feedback for consideration by the BigBone project.

[bocops]

Issue 1:

Regarding the first issue, I introduced a nullable Boolean in our Status entity based on this Mastodon documentation for the pinned value:

If the current token has an authorized user: Have you pinned this status? Only appears if the status is pinnable.

Compared to, for example, bookmarked, which has this definition:

If the current token has an authorized user: Have you bookmarked this status?

I believe we can just simplify this to a non-nullable Boolean, in which case it would just be false for most statuses, including those that can not be pinned. In Java, this would become a boolean primitive just like all the others.

If we do that, library users must not use the existence of this non-null value to check if a status can be pinned, so a Kdoc should be added stating that - but other than that, we should be safe.

Issue 2:

Regarding the second issue, I'm not exactly sure how to proceed. I believe it is correct for us to use an enumeration instead of a String that either we or each library user would need to check for correctness in our postStatus() call. I think it is also necessary for our Status entity to have its visibility defined as a String for deserialization of the Mastodon server response.

Possible solutions include:

1. add a method getVisibilityConstant() to our Status entity, that returns the proper Visibility based on the visibility string value.
2. add a method fromValueString(string) to our Visibility enum, that returns a Visibility based on the parameter, or null if the parameter is invalid
3. add a companion object method (static in Java) getVisibilityFromString(string) to the Status entity class that works as above
4. add this method somewhere else completely
5. overload postStatus(), so that it accepts a String instead of a Visibility.

I don't like the last idea of overloading a post method - but the rest should be equally possible, while each of them comes with different pros and cons. @andregasser, do you have any preference?

[bocops]

@MastodonContentMover a change for issue 1 above just went in - so, starting with the next snapshot, checking s.isPinned() != null should no longer be necessary.