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

Introduce builders where appropriate #108

Open
andregasser opened this issue Jan 18, 2023 · 1 comment
Open

Introduce builders where appropriate #108

andregasser opened this issue Jan 18, 2023 · 1 comment
Labels
breaking Incompatible with previous versions code quality Everything related to code quality
Milestone

Comments

@andregasser
Copy link
Owner

andregasser commented Jan 18, 2023

There are cases where issuing a request using our client becomes a bit clunky. one example is when we want to post a status by calling the postStatus(...) method.

Example code taken from PostStatusWithMediaAttached.java in the sample-java collection:

// Post status with media attached
final StatusesMethods statusesMethods = new StatusesMethods(client);
final String statusText = "Status posting test";
final String inReplyToId = null;
final List<String> mediaIds = Collections.singletonList(mediaId);
final boolean sensitive = false;
final String spoilerText = "A castle";
final Visibility visibility = Visibility.Private;
statusesMethods.postStatus(statusText, inReplyToId, mediaIds, sensitive, spoilerText, visibility).execute();

The method signature of postStatus looks like this:

@JvmOverloads
@Throws(BigBoneRequestException::class)
fun postStatus(
    status: String, 
    inReplyToId: String?, 
    mediaIds: List<String>?, 
    sensitive: Boolean, 
    spoilerText: String?, 
    visibility: Status.Visibility = Status.Visibility.Public
): MastodonRequest<Status>

postStatus() uses the @JvmOverloads annotation. status and sensitive are non-nullable whereas the others are. So when we want to call this method from Java, we need to pass a value for inReplyToId even it is an optional field. This is because the amount of method overloads generated by @JvmOverloads is limited. By using a builder, we could avoid passing null values just to make the method call work. The whole call would look cleaner and be more readable:

final StatusesMethods statusesMethods = new StatusesMethods(client);
var status = Status.Builder("Status posting test")
    .addMediaId(mediaId)
    .sensitive(false)  // could be even left away if false is a default value
    .spoilerText("A castle")
    .visibility(Visibility.Private)
    .build()
statusesMethods.postStatus(status).execute();

Required attributes would go into the Builderconstructor, whereas optional arguments can be specified using builder methods.

But: Naming must be clarified, as Status is already used as a response entity from the Mastodon API...

@andregasser andregasser added the code quality Everything related to code quality label Jan 18, 2023
@bocops
Copy link
Collaborator

bocops commented Jan 30, 2023

I just commented in #124 that both editing an existing status, as well as deleting&redrafting a status, are actions that are similar to posting a status and could probably use a builder as well.

At least the delete&redraft action could probably even be solved using the same builder, by giving that a .fromStatus(status) method that fills necessary from the Status entity that was returned when deleting the old status.

@andregasser andregasser added this to the 3.0.0 milestone Feb 28, 2023
@andregasser andregasser added the breaking Incompatible with previous versions label Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Incompatible with previous versions code quality Everything related to code quality
Projects
None yet
Development

No branches or pull requests

2 participants