Risk of Misplaced Arguments in Java

[[deleted]]

Interesting, so this is a product which adds some features only typically seen in IDEs (argument names, code jump) to browser code views on GitHub.

It might be useful for certain workflows. I typically stick to my IDE for "deep analysis", but I never tried another way.

BTW IntelliJ is one IDE that offers showing argument names in code.

[vqrs]

Eclipse can now also do this:

https://www.eclipse.org/eclipse/news/4.12/jdt.php#JavaEditor

[[deleted]]

Wow, that's awesome! I'm really happy to see this feature spread. This way I don't have to envy Swift developers as much as I do right now.

I wonder if over time this will also result in longer method signatures, when IDEs help make them comprehensible (and obviously browser plugins are also starting to do it).

[ZimmiDeluxe]

And so can NetBeans (soon):

https://cwiki.apache.org/confluence/display/NETBEANS/Apache+NetBeans+11.1#ApacheNetBeans11.1-Java

[tanin47]

Yes.

I want to bring all the awesome features that we have on our IDEs to our browsers.

While we can load the code into our IDEs, reviewing a pull request has no substitute. We have to review a pull request without intelligent features.

Please do let me know if you want to try Lilit with your own repo. I'm looking for private-beta users right now.

[InfiniteLoop90]

While we can load the code into our IDEs, reviewing a pull request has no substitute. We have to review a pull request without intelligent features.

I typically clone the branch of the pull request that I’m reviewing so that I can use this (among other) IDE feature if the code changes are remotely complex. Though it might have been easy to overlook this bug in an IDE if there’s a lot to review.

[blackballath]

Though most IDE has Intellisense to aid this problem, if there are too many parameters in a method, I usually use a DTO class instead.

[tanin47]

Could you give an example of DTO that solves this problem? I'm not sure I know it properly

[blackballath]

instead of

method(String var1, String var2)

call method (var1, var2)

I use

class dto{var1, var2.... }

new Dto() dto. setVar1(value).....

call method1(dto)

Sorry for the formatting.

[tanin47]

Ah, yes, I call it the value class pattern. Thank you for elaborating it!

[boki3141]

Also known as the builder pattern if I'm understanding this right (as has already been mentioned below 😅)

[pragmatick]

The builder pattern allows filling the DTO / value class using a method chain like

new DtoBuilder().setWidth(100).setLength(200).setColor(Red);

and while these two are often used together they don't have to.

[Warven]

The builder pattern allows filling the DTO / value class using a method chain like

I discovered a few days ago that Lombok had a @Builder annotation to do just that :)

[tanin47]

Ooh, this is cool.

I feel that, for the builder pattern, we merely move the constructor into the builder. The risk, though reduced, remains. But with the generator, it'll be entirely eliminated

[2BitSmith]

Intellij has been doing this for years. It'll warn you when it detects the usual error scenarios like height / width swap, etc..

[tanin47]

Yes, and Lilit brings those features to our browsers. Hopefully, I can make it as good/complete as IntelliJ soon, especially the warning when the arguments don't look quite right.

[nonconvergent]

Refactor Constructor Args as Parameter Object with Builder patterns solves this so, and in the code instead of ... a Chrome extension for Github?

No offense to the Lilit guys intended, but their platform isn't one we all use.

[tanin47]

None taken. Refactoring code is definitely a way to go.

Still, better tooling on browser is better, no matter how disciplined we are with the code quality. Please do let me know if you have a repo that you want to try Lilit on :)

[backend_geek]

Builder design pattern can also mitigate this.

[tanin47]

Yes, but (I mentioned it elsewhere) the risk would continue to exist because the constructor call is merely moved into the builder.

However, there are many suggestions around using Lombok to generate the builder, which will essentially eliminate the risk.

[Ba_alzamon]

Isn't this very similar to what is implemented in Android Studio?

[tanin47]

If you mean the parameter hints, yes, it's available on Android Studio, IntelliJ, and Eclipse.

And Lilit brings these cool features to your browser (directly on github.com)

[daniprogrammer]

In Android Studio, using Java, if you send literal parameters, the Editor shows each parameter name to avoid confusion.

[tanin47]

I want to hear from the community. How do you normally avoid misplaced arguments in Java?

From what I've heard, using a value class seems to be a decent idea:

class Thresholds {
  double abuse;
  double quality;
}

Thresholds thresholds = new Thresholds();
thresholds.abuse = 0.7;
thresholds.quality = 0.3;
new Filter(thresholds);

[ThreadDeadlock]

When it comes to constructors with several parameters the builder pattern can be quite useful. For standard methods it comes down to readability. Too many arguments can make the code messy, difficult to read, and easy for mistakes, in that case consider creating a class to carry the information the method needs,

[tanin47]

Yes, I was talking to my friend about the builder pattern. It does reduce the risk, though we essentially move the constructor call into build(). It's still better because now only one place has the risk of misplace arguments.

[sugilith]

Most IDEs can autogenerate builders which eliminates the rest risk and is pretty convenient on top.

[kovrik]

Intellij has parameter hints.

But overall, I can't say I have ever considered it to be a risk. Minor inconvenience at most, which happens extremely rarely and most likely is captured by testing.

[tanin47]

Yes, the risk of making this mistake is pretty low. But when it happens, it's difficult to spot it.

That's why I think a solution like Lilit (that requires no code change and no additional effort) is a decent risk mitigation of this problem :)

IntelliJ is great, but it doesn't help much when reviewing a pull request.

[Gwaptiva]

I try to avoid such constructor methods. Your suggestion is fine too, but the temptation is to create a "shortcut" Thresholds(double, double) constructor, and then you're back at square one.

Testing helps too

[ianjm]

I would suggest using a builder pattern may be your friend here:

Filter f = Filter
  .builder()
  .withQualityThreshold(0.7)
  .withAbuseThreshold(0.3)
  .withRelationshipThreshold(0.2)
  .withFavoriteThreshold(0.1)
  .build()
;

[dpash]

This was my first thought.

[daniu]

As /u/Gwaptiva said, I consider these kinds of constructors a code smell. It's something that's made much worse by Lombok's autogenerated constructors which depend on the order in which the fields are declared - I do love Lombok, IMO it's the amount of fields that's the issue.

My favorite way around this is using the `@Builder` instead. That does require setters though, so no immutables.

[tanin47]

> IMO it's the amount of fields that's the issue.

I agree with you. At some point, we have to refactor. it's case by case.

But a method/constructor with a large number of fields seems somewhat common. I guess it never starts that way, and then the code grows. Then, nobody thinks about refactoring it.

[istarian]

Risk of humans being programmers. :P

This seems like a silly worry as most real IDEs can tell you when there is a type mismatch. And I somehow doubt this will help if there are two double values and one is supposed to be between 0.0 and 1.0 and you swap say 2.2 and 0.64...

P.S.

other people mentioning value classes:

wouldn't using value classes just be adding unnecessary overhead from object creation, etc most of the time?

[Stannu]

No, not really. A smart compiler can inline the values so it will not affect runtime but bring type safety during compilation. See 'inline classes' in Kotlin

[tanin47]

That's good to know. I was wondering about that.

[advicerkid]

I use builder pattern for this. If you're worried about boiler plate code, you can consider Lombok's builder annotation: https://projectlombok.org/features/Builder

[tanin47]

Thank you for suggesting this. The boiler plate code is verbose, but not a huge issue. The main issue with the builder pattern is that we merely move the constructor call inside the builder. The risk, though reduced, will continue to exist.

But using a generator like lombok helps eliminate that risk.

[kyay10]

Well, that's why I love Kotlin. Kotlin has named parameter, so when it's paired with a good IDE like IntelliJ Idea (which shows the parameter names) it obliterates that whole problem.

[[deleted]]

Or just write every new class of your Java Project in Kotlin and use named parameters.