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

Automatic code formatting #846

Open
abyrd opened this issue Dec 21, 2022 · 0 comments
Open

Automatic code formatting #846

abyrd opened this issue Dec 21, 2022 · 0 comments

Comments

@abyrd
Copy link
Member

abyrd commented Dec 21, 2022

We would like to apply uniform formatting rules and regularly check that they're applied. These can be applied by command line tools or IDE plugins. We can collect some observations here before settling on a method.

In practice, most sets of formatting rules I've worked with in the past made certain code much messier or harder to read, or just failed to work properly. Hopefully the situation has changed.

We'd want to ensure all big whitespace changes are applied in separate commits from intentional code commits. Grouping whitespace changes together with code changes in the same commit, or even the same set of commits, can make it hard to review changesets. Maybe we could reformat a few key files in individual commits, and if we like the style we occasionally do project-wide or full-file reformats to get it out of the way.

One such tool we're trying out is https://github.com/google/google-java-format. There's an IntelliJ plugin described on the Github landing page.

Some initial observations from applying it to whole files:
It looks good overall but the line width and two-space indent are a significant departure from what we've been using (and what is standard in a lot of other Java projects). Some of the punctuation and wrapping looks less clear to me.

A few observations:

Aligned assignments are wiped out:

        calendar.monday    = timetable.monday    ? 1 : 0;
        calendar.tuesday   = timetable.tuesday   ? 1 : 0;
        calendar.wednesday = timetable.wednesday ? 1 : 0;
        calendar.thursday  = timetable.thursday  ? 1 : 0;
        calendar.friday    = timetable.friday    ? 1 : 0;
        calendar.saturday  = timetable.saturday  ? 1 : 0;
        calendar.sunday    = timetable.sunday    ? 1 : 0;

becomes

    calendar.monday = timetable.monday ? 1 : 0;
    calendar.tuesday = timetable.tuesday ? 1 : 0;
    calendar.wednesday = timetable.wednesday ? 1 : 0;
    calendar.thursday = timetable.thursday ? 1 : 0;
    calendar.friday = timetable.friday ? 1 : 0;
    calendar.saturday = timetable.saturday ? 1 : 0;
    calendar.sunday = timetable.sunday ? 1 : 0;```

Some indentation seems inconsistent, with some things indented by 4 spaces and others by two:

              TripSchedule sched =
                  TripSchedule.create(
                      trip,
                      arrivals,
                      departures,
                      frequencies,
                      IntStream.range(0, arrivals.length).toArray(),
                      serviceCode);
              if (frequency) {
                timetable.applyPhasing(sched);
              }

I'm not sure if this is a mistake or intentional. Over the last year or so I've also come to like treating closing parens on multi-line method calls the same way as closing curly brackets:

              TripSchedule sched =
                TripSchedule.create(
                  trip,
                  arrivals,
                  departures,
                  frequencies,
                  IntStream.range(0, arrivals.length).toArray(),
                  serviceCode
                );
              if (frequency) {
                timetable.applyPhasing(sched);
              }

To me it would be a shame to lose things that have been nicely hand-formatted like the above.

Obviously in the big picture consistency is more important than bikeshedding any one detail, but I'd like to try a couple of other formatters before settling on one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant