• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Code formatting

 
Oliver Weikopf
Ranch Hand
Posts: 58
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello everyone,

I'm currently leafing through my code, adding javadoc comments, looking for overly complicated code, etc.

My questions:
  • My main class for the application is currently at 920 lines. This includes comments and several inner classes. These inner classes are dialogs for searching, booking, and settings as well as a class extending Action. I've taken a long hard look at them and they seem to make more sense as inner classes than as public classes (this would require some awkward parameter passing). These inner classes make up a total of 330 lines. Do you think this is acceptable or do I have to pull out the inner classes? 920 lines seems a bit too long to me. Is there something like a limit that should not be exceeded per file?
  • I've added javadoc comments for nearly all private fields and methods. Is that overdoing it? I'm not sure since javadoc comments are only required for public methods and fields and not all of the private fields really require comments to understand them.
  • I've hard-coded all labels, button names, mnemonics, dialog texts etc. into the application. It's something I'd never do with a "real" application, mind you. Do you think that's ok or should I approach this using ResourceBundles as language files (or some other way of not-hardcoding text)?
  • I've also hard-coded values like the window size, debug mode, The database file cookie value, column names, table column widths. Same question here.
  • What number of characters per line do you think is acceptable? I've got lines with a length of up to 305 characters.

  •  
    Jeroen T Wenting
    Ranch Hand
    Posts: 1847
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    1) Any inner class that's non-trivial is a candidate for turning into a toplevel class at package level.
    2) You can never have too much (relevant and correct) documentation.
    3) No internationalisation is required, the extra effort needed would be overkill.
    4) ditto. You should probably pull them to be static final values though, instead of scattering them through the code of your classes.
    5) 80, or a little more if it makes for a more logical break in your lines.

    I suggest you read the Sun coding conventions, which will tell you what your code should look like.
    Not following those conventions can lead to serious point loss, and possibly failure (you wouldn't want to loose all points in 2 categories for not writing code that looks like Sun wants it to look, which could theoretically happen if you divert badly enough).
     
    Mihai Radulescu
    Ranch Hand
    Posts: 918
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Take a look at :

    http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html

    Regards M.
     
    Oliver Weikopf
    Ranch Hand
    Posts: 58
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thanks, Jeroen and Mihai, this has been very helpful. Seems like I'll have a lot of wrapping to do with a line length maximum of 80 characters.

    The only thing I couldn't find in there is a maximum number of lines per java file. I know I once worked on a project where a file was not allowed to have more than 300 or 400 lines. How many lines do your files have?
     
    Mihai Radulescu
    Ranch Hand
    Posts: 918
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Oliver

    I think that 2000(comments are also counted) lines is reasonable.

    regards M
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12007
    215
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Originally posted by Mihai Radulescu:
    I think that 2000(comments are also counted) lines is reasonable.
    Wow - I generally hate to see any file exceed 1000 lines, and rarely even get to half of that. I think if you are getting to 2000 lines you may have your class doing too much.

    To give an example, to write the code for our book, which in many cases is more complex than the real assignment, we didnt hit 500 lines in any class:Maximum was on the DVD class which hit 476 lines!

    Regards, Andrew
     
    Oliver Weikopf
    Ranch Hand
    Posts: 58
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Originally posted by Andrew Monkhouse:
    Maximum was on the DVD class which hit 476 lines!

    Regards, Andrew


    Wow! That's 44 classes. I've got 17 top-level classes (plus 5 inner classes). Hopefully that doesn't mean I'm way off. Also, I've got a total of 2570 lines. At least that number will increase significantly when I start reducing to 80 characters per line.
     
    Jeroen T Wenting
    Ranch Hand
    Posts: 1847
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Not quite up to date (it's an older version I've not updated with the latest changes yet):

    0 client/CVS
    0 db/bs
    0 db/client
    0 db/CVS
    0 db/net
    0 icons/CVS
    0 server/CVS
    0 utils/CVS
    1 CVS/Repository
    1 CVS/Root
    4 icons/Book16.png
    4 icons/search16.png
    7 CVS/Entries
    8 icons/quit16.png
    9 utils/package.html
    10 client/package.html
    12 server/Server.jbx
    13 db/package.html
    14 client/Subcontractor.jbx
    14 server/package.html
    27 db/Finder.java
    28 client/ServerOptions.jbx
    29 db/SearchType.java
    31 db/FieldType.java
    34 db/InvalidFileFormatException.java
    43 server/serverhelp.html
    48 db/DecoratedDB.java
    50 db/DuplicateKeyException.java
    51 db/RecordNotFoundException.java
    51 db/ValidateException.java
    52 db/DatabaseException.java
    53 client/AboutBoxLauncher.java
    57 client/ReadOnlyTable.java
    57 utils/HelpLauncher.java
    64 client/Client.java
    64 client/HighlightTableCellRenderer.java
    68 client/ConnectAction.java
    68 utils/HelpFrame.java
    71 db/MetaData.java
    74 client/DisconnectAction.java
    75 client/AbstractSearchDisplay.java
    78 client/SubContractorTable.java
    78 utils/FramePosition.java
    87 client/QuitAction.java
    91 client/TableEventHandler.java
    94 client/clienthelp.html
    98 client/BookAction.java
    102 client/NameCitySearchDisplay.java
    116 client/SubcontractorDisplay.java
    122 db/DB.java
    127 db/FinderFactory.java
    131 server/Engine.java
    133 client/SubcontractorTableModel.java
    133 utils/ConfigManager.java
    135 client/SearchAction.java
    147 db/Field.java
    189 client/SearchDlg.java
    193 server/Server.java
    251 client/RecordDetails.java
    277 db/LockManager.java
    280 db/DataFile.java
    293 client/Subcontractor.java
    298 db/Data.java
    325 client/DatabaseActionHandler.java
    357 client/ServerOptions.java
    411 server/MainFrame.java
    455 client/MainWindow.java
    6193 total

     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic