Unreadable imports

[matthias-ronge]

I notice that the list of imports at the beginning of all classes is extremely long and impossible to keep track of, especially in reviews. Therefore, the technical suggestion I would like to make here is that we define the imports in packages (and identify ambiguities if necessary) instead of just taking one thing out of each package at a time. That means, for example, instead of:

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.Serializable;
import java.text.DecimalFormat;
import java.time.DateTimeException;
import java.time.LocalDate;
import java.time.Month;
import java.time.MonthDay;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.faces.context.FacesContext;
import javax.faces.view.ViewScoped;
import javax.inject.Named;
import javax.naming.ConfigurationException;
import javax.xml.transform.TransformerException;

import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.commons.lang3.tuple.Triple;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.kitodo.config.ConfigCore;
import org.kitodo.config.enums.ParameterCore;
import org.kitodo.data.database.beans.Process;
import org.kitodo.data.database.exceptions.DAOException;
import org.kitodo.data.exceptions.DataException;
import org.kitodo.exceptions.DoctypeMissingException;
import org.kitodo.exceptions.ProcessGenerationException;
import org.kitodo.production.forms.createprocess.ProcessDetail;
import org.kitodo.production.forms.createprocess.ProcessTextMetadata;
import org.kitodo.production.helper.Helper;
import org.kitodo.production.helper.XMLUtils;
import org.kitodo.production.helper.tasks.GeneratesNewspaperProcessesThread;
import org.kitodo.production.helper.tasks.TaskManager;
import org.kitodo.production.model.bibliography.course.Block;
import org.kitodo.production.model.bibliography.course.Cell;
import org.kitodo.production.model.bibliography.course.Course;
import org.kitodo.production.model.bibliography.course.Granularity;
import org.kitodo.production.model.bibliography.course.IndividualIssue;
import org.kitodo.production.model.bibliography.course.Issue;
import org.kitodo.production.model.bibliography.course.metadata.CountableMetadata;
import org.kitodo.production.process.NewspaperProcessesGenerator;
import org.kitodo.production.services.ServiceManager;
import org.kitodo.production.services.calendar.CalendarService;
import org.kitodo.production.services.data.ImportService;
import org.primefaces.PrimeFaces;
import org.primefaces.model.DefaultStreamedContent;
import org.primefaces.model.StreamedContent;
import org.primefaces.model.file.UploadedFile;
import org.w3c.dom.Document;
import org.xml.sax.SAXException;

Let's specify:

import java.io.*;
import java.text.*;
import java.time.*;
import java.util.*;
import java.util.regex.*;

import javax.faces.context.*;
import javax.faces.view.*;
import javax.inject.*;
import javax.naming.*;
import javax.xml.transform.*;

import org.apache.commons.lang3.tuple.*;
import org.apache.logging.log4j.*;
import org.kitodo.config.*;
import org.kitodo.config.enums.*;
import org.kitodo.data.database.beans.Process;
import org.kitodo.data.database.exceptions.*;
import org.kitodo.data.exceptions.*;
import org.kitodo.exceptions.*;
import org.kitodo.production.forms.createprocess.*;
import org.kitodo.production.helper.*;
import org.kitodo.production.helper.tasks.*;
import org.kitodo.production.model.bibliography.course.*;
import org.kitodo.production.model.bibliography.course.metadata.*;
import org.kitodo.production.process.*;
import org.kitodo.production.services.*;
import org.kitodo.production.services.calendar.*;
import org.kitodo.production.services.data.*;
import org.primefaces.*;
import org.primefaces.model.*;
import org.primefaces.model.file.*;
import org.w3c.dom.*;
import org.xml.sax.*;

The idea behind the imports list is (or should be)—at least that's how I learned it—to give the reader an overview of what a class interacts with. If we list packages, it's much easier to understand.

[henning-gerhardt]

I can understand your suggestion but I don't know if this change is more readable then before. I feel the lost of which explicit class of a package is used. I did not look at first at the used package I look first on the used class.

If there is same named class (f.e. List or Array) in at least two or more packages than a explicit import rule must be used. I don't know how many times this will happen but it can confuse readers too as the import schema is different at "random" places.

But this is only my opinion. If other (active) developers agree with your change, then go for it as I life with this change.

[solth]

I agree with @henning-gerhardt and think that individual imports actually improve the readability. Wildcard imports are also discouraged because they tend to load too many classes that are not required and mask which part of a package is actually used in the code, as far as I know. Individual imports also act as some form of code documentation because its always clear what package a class comes from (an example that comes to mind is Process, which could either come from Java or from Kitodo).

I think that is also the reason why we once added a rule to the Kitodo coding guidelines to always use individual imports over wildcard imports.

And to be fair, the code you shared above is quite an extreme example. Most classes in Kitodo have way fewer import statements than that. Altogether I would be in favour of keeping our current approach with individual imports.