NetLogo/NetLogo — pull request #50 by TheBizzle (2012-01-10 17:15:37+00:00)
Title: Import Optimization Pull Request (Finally)

Sorry that took so long.... I've never actually forked, made a patch through Git, applied a patch through Git, _or_ done a pull request before, so... that was somewhat of a learning experience for me. After that, I then had to rerun the build to make sure that everything was alright.

Anyway, some things that you should know about this:

-IntelliJ got a little ambitious and altered some non-src files. I reverted those files back.

-Now that I can see the changes in GitHub, I notice that quite a few of these changes take the mostly-irrelevant form of

import A
import C
import B

changing to

import A
import B
import C

or the form of

import A.first
import A.second
import A.third
import A.fourth

changing to

import A.* / import A._

-Also, there were a a handful files wherein IntelliJ made some mistakes on things that had form

import A.{first, second, third}
import third.alpha

It would remove the import of A.third and be all confused about why that then wouldn't compile properly. I went through and manually fixed those.

-It also did some bad refactoring of imports on two or three particular abstract Java classes that imported a large number of things. I manually reverted those, as well.

-After forking, when I applied the patch and tried to compile/run NetLogo, I got some "nonexhaustive match" errors. ViewControlToolBar was supposedly nonexhaustively matching on org/nlogo/api/Perspective. I then "fixed" Perspective by changing its class template and usage to be what I'm pretty sure the writer of Perspective was _actually_ going for, and that collection of match errors went away. I think you might want my changes to this file, regardless of what you think of the import "optimization", because... either I'm _really_ not getting what you guys were going for there, or there was a bit a minor bug in that class.

-I also ran into nonexhaustive match errors in LocalFile and RemoteFile, when they were matching on org/nlogo/api/FileMode. I "fixed" this by turning FileMode from a "abstract (redundant) sealed trait" to just being a sealed abstract class that took a single Int for its constructor (like how I made Perspective to be). This resolved all of the remaining nonexhaustive match errors that I was getting. Naturally, this could also be solved by marking the matches with the unchecked annotation, or just ignoring my pull request entirely.

-With the number of errors that it caused initially, and with the somewhat low signal-to-noise ratio of ACTUAL meaningful fixes that it made, I don't really plan on running that "optimize imports" feature anytime soon. It's been mildly helpful for me in the past, but... overall, I don't think it was it was worth it for this. I'm guessing that it found maybe... 100 ACTUAL unused imports that it got rid of. If that's all it really found, ignoring unused imports for the next 10 years seems to be a viable enough strategy to me....