0
\$\begingroup\$

I have the situation, where I have to methods in my api calling class that do basically the same thing. I have a profile that contains multiple account objects. These objects have their own UUID as a string and the profile has a unique Integer value as an identifier. I want to get the object once by its profile id and once by one of its accounts ids.

    public ProfileViewData getGGAuthProfileView(Integer authId) {
        return (ProfileViewData) apiService.makeRequest(
                "/api/profile/ggauth/" + authId,
                RequestMethod.GET,
                ProfileViewData.class);
    }

    public ProfileViewData getGGAuthProfileViewByMinecraftUUID(String minecraftUUID) {
        try {
            return (ProfileViewData) apiService.makeRequest(
                    "/api/profile/getauthprofilebyminecraftuuid/" + uuid.toString(),
                    RequestMethod.GET,
                    ProfileViewData.class);
        } catch (DataNotFoundException e) {
            return null;
        }
    }

This is what I have and it works fine, but I really dislike the way the code is structured right now. I have doubts about naming the method exactly the same and having only different input types because that would require an unexperienced user of my code to understand what the different types of the argument imply. This can be ok if the arguments are named correctly, but I am not convinced of this approach.

What to do with this? I can rename the second endpoint, the first one unfortunately cannot be changed without a lot of effort not only from my part.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Nothing wrong about being explicit. I'd probably just remove the "get" from the path, as that's already implied by the http method. \$\endgroup\$
    – slepic
    Jun 10, 2023 at 6:03
  • \$\begingroup\$ Incorporating advice from an answer into the question violates the question-and-answer nature of this site. You could post improved code as a new question, as an answer, or as a link to an external site - as described in I improved my code based on the reviews. What next?. I have rolled back the edit, so the answers make sense again. \$\endgroup\$ Jun 10, 2023 at 7:37
  • \$\begingroup\$ I thought more of a "pressed send too early" case, but thanks for the advice :) \$\endgroup\$
    – monamona
    Jun 10, 2023 at 8:54

2 Answers 2

2
\$\begingroup\$

First, in Java, overloaded methods (same method name but different parameters) are common. In cases where the reason for the overloaded method may not be clear, there's documentation, especially javadoc that can be viewed in-line in most IDEs.

Second, like overloaded methods that have the same name and return the same thing, I'd expect API endpoints that return the same data to have similar names. In this case, it looks like the profile can be fetched by ggauth or minecraftUUID. Based on that, API endpoints "/api/profile/ggauth/" + authId and "/api/profile/minecraftuuid/" + uuid make sense. (This would also make the methods getProfileView(Integer ggAuthId) and getProfileView(String minecraftUUID).)

\$\endgroup\$
3
  • \$\begingroup\$ Oh... yeah, I copied a version of the code that had that bug in it. Edited the question! \$\endgroup\$
    – monamona
    Jun 10, 2023 at 6:27
  • \$\begingroup\$ Okay... the main point of my question was if this is enough to properly follow good code principles. I usually try to write code that explicitly is not dependent on javadocs, since they are not always properly used by other developers and I still feel like they get sometimes just lost. However, the suggestion for the api name and the method names is very welcome! :) \$\endgroup\$
    – monamona
    Jun 10, 2023 at 6:30
  • \$\begingroup\$ I agree javadocs are inconsistently used by developers, though the tools more consistently support them now. It seemed the potential confusion was driven by including one of the parameters (ggauth) in both method names, hence my suggestion. :) \$\endgroup\$
    – Naomi R
    Jun 10, 2023 at 6:41
1
\$\begingroup\$

overloading can be a solution of your use-case, BUT, make sure that for example If you want to create unit tests, and passing null for some tests to this method

getProfileView(null) 

the compiler will prompt an error for ambiguity method.

so what I suggest is to overload int with primitive type If it's possible.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.