nus-cs2103-AY2425S1 / forum

12 stars 0 forks source link

Coding standard for custom import statements #305

Closed gohsl99 closed 1 month ago

gohsl99 commented 1 month ago

The system import statements are separated by their groups / packages in coding standard, should we do the same for custom import statements, i.e. follow option 1 instead of option 2?

Option 1:

import java.io.File;
import java.io.FileWriter;

import myfirstpackage.Main;
import myfirstpackage.Second;
import myfirstpackage.Third;

import mysecondpackage.Main;
import mysecondpackage.Second;

import mythirdpackage.Main;

Option 2:

import java.io.File;
import java.io.FileWriter;

import myfirstpackage.Main;
import myfirstpackage.Second;
import myfirstpackage.Third;
import mysecondpackage.Main;
import mysecondpackage.Second;
import mythirdpackage.Main;
emmannyyy commented 1 month ago

I believe option 1 may be neater. Just wondering, does Checkstyle show any violation?

Also, you can probably get intellij to configure this for you automatically like #132 suggests :)

damithc commented 1 month ago

Also, you can probably get intellij to configure this for you automatically like #132 suggests :)

Yup, this guide is aligned with the import order followed by AB3 https://se-education.org/guides/tutorials/intellijCodeStyle.html

gohsl99 commented 1 month ago

All imports for my files are using option 1 but I am getting checkstyle error, e.g.

[ant:checkstyle] [ERROR] C:\Directory\Parser.java:17:1: Extra separation in import group before 'exception.BlitzCommandDoesNotExistException' [CustomImportOrder]

My import look something like this: image

Do I need to change the checkstyle rule or something?

gohsl99 commented 1 month ago

Also, you can probably get intellij to configure this for you automatically like #132 suggests :)

Yup, this guide is aligned with the import order followed by AB3 https://se-education.org/guides/tutorials/intellijCodeStyle.html

Hi Prof, it has been 2 days and no one else has a answer for my last comment. May I just clarify do we stick to https://se-education.org/guides/tutorials/intellijCodeStyle.html strictly, i.e. I should not put blank lines between custom import (to separate the imports by my packages) because these imports belongs to "all other imports"? If I follow the style strictly, is it fine if my custom imports look like this (note, these imports are in my Parser class, which requires all command objects and handle exceptions):

import command.ByeCommand;
import command.Command;
import command.DeadlineCommand;
import command.DeleteCommand;
import command.EventCommand;
import command.FindCommand;
import command.ListCommand;
import command.MarkCommand;
import command.TodoCommand;
import command.UnmarkCommand;
import exception.BlitzCommandDoesNotExistException;
import exception.BlitzException;
import exception.BlitzInvalidParameterMoreThanOneException;
import exception.BlitzInvalidParameterRegexException;
import exception.BlitzNoParameterException;
damithc commented 1 month ago

Hi Prof, it has been 2 days and no one else has a answer for my last comment. May I just clarify do we stick to https://se-education.org/guides/tutorials/intellijCodeStyle.html strictly, i.e. I should not put blank lines between custom import (to separate the imports by my packages) because these imports belongs to "all other imports"? If I follow the style strictly, is it fine if my custom imports look like this (note, these imports are in my Parser class, which requires all command objects and handle exceptions):

@gohsl99 Yes, the easiest solution seems to be to just remove the blank line. But you may change the checkstyle configuration too, if you wish. If you do the latter, you are likely to run into the same issue in the tP, and you'll need the team's consensus to change the checkstyle config.

gohsl99 commented 1 month ago

Hi Prof, it has been 2 days and no one else has a answer for my last comment. May I just clarify do we stick to https://se-education.org/guides/tutorials/intellijCodeStyle.html strictly, i.e. I should not put blank lines between custom import (to separate the imports by my packages) because these imports belongs to "all other imports"? If I follow the style strictly, is it fine if my custom imports look like this (note, these imports are in my Parser class, which requires all command objects and handle exceptions):

@gohsl99 Yes, the easiest solution seems to be to just remove the blank line. But you may change the checkstyle configuration too, if you wish. If you do the latter, you are likely to run into the same issue in the tP, and you'll need the team's consensus to change the checkstyle config.

Alright understood, thank you Prof!