MagicNumber usage in GUI code

30 views

Skip to first unread message

Roman Ivanov

unread,

Oct 2, 2015, 10:03:52 AM10/2/15

to checkstyle-devel

In a team we had an non finished discussion about MagicNumber usage in all source without any suppression.

Checkstyle project has suppression:

https://github.com/checkstyle/checkstyle/blob/master/config/suppressions.xml#L43

https://github.com/checkstyle/checkstyle/blob/master/config/suppressions.xml#L63

Question: Should we keep suppression or we should follow Check recommendations blindly and name all numbers by variables.

My position:

First and foremost , do not be fanatic to some rules, keep common sense and readability of code above recommendations.

Most recommendations are generic and can not be applied to all code in the world.

Secondly, src/test - we need to review and I am ok to fix, we just need to review. Suppression was set to let focus on main code cleanup and postpone test code cleanup for later time.

Third, GUI. My position is that all gui projects should suppress that Check completely of in gui packages only as we do.

GUI code is full of custom numbers that are clear only for UI engineers as it is nuances of layout. Naming all of them do not make sense and

create one more level of abstraction for engineer.

Lets take a look at example:

First swing project from github - https://github.com/adrian/upm-swing/tree/master/src/com/_17od/upm/gui

(just a simple UI code, we could find more thick(a lot of controls) UI files)

/var/tmp $ git clone https://github.com/adrian/upm-swing.git

06:44 /var/tmp $ cat config.xml

<?xml version="1.0"?>

<!DOCTYPE module PUBLIC

"-//Puppy Crawl//DTD Check Configuration 1.3//EN"

"http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

</module>

06:44 /var/tmp $ java -jar checkstyle-6.11.1-all.jar -c config.xml upm-swing/src/com/_17od/upm/gui/

Starting audit...

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:313:73: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:314:23: error: '60' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:314:28: error: '1000' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:893:50: error: '1000' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:896:74: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:896:78: error: '60' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabaseActions.java:896:83: error: '1000' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:63:65: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:63:68: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:63:71: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:83:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:91:39: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:95:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:95:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:95:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:107:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:107:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:115:44: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:117:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:119:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:119:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:119:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:129:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:131:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:131:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:139:48: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:141:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:143:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:143:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:143:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OpenDatabaseFromURLDialog.java:152:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:67:65: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:67:68: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:67:71: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:87:34: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:99:34: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:99:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:99:40: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:111:31: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:111:34: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:126:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:128:34: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:128:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:128:40: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/DatabasePropertiesDialog.java:138:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:93:65: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:93:68: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:93:71: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:116:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:116:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:124:115: error: '25' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:129:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:129:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:129:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:145:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:145:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:157:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:157:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:181:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:183:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:183:37: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:183:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:194:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:196:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:207:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:209:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:225:51: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:227:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:229:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:229:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:240:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:242:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:252:56: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:254:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:256:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:256:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:267:27: error: '7' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:269:45: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:298:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:329:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:341:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:341:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:353:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:353:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:353:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:361:105: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:365:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:365:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:372:105: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:376:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:376:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:376:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:386:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:388:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:388:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:396:113: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:398:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:400:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:400:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:400:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:410:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:412:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:412:37: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:425:65: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:427:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:429:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:429:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:429:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:448:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:450:34: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/OptionsDialog.java:450:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:149:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:149:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:149:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:149:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:159:77: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:166:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:191:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:211:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:221:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:221:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:221:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:221:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:233:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:233:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:233:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:233:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:243:67: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:250:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:275:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:295:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:305:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:305:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:305:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:305:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:317:31: error: '15' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:317:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:317:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:317:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:327:75: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:339:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:356:107: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:359:72: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:384:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:404:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:421:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:424:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:433:51: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:433:57: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:462:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:462:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:462:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:462:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:472:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:474:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:474:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:474:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:474:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:484:61: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:491:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:529:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:549:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:566:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:569:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:577:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:579:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:579:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:579:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:579:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:589:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:591:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:591:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:591:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:591:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:601:64: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:601:68: error: '20' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:611:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:631:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:651:40: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:659:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:661:31: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:661:35: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:661:39: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:661:43: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:671:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:676:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:699:19: error: '6' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:701:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:701:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AccountDialog.java:704:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:205:52: error: '1.4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:238:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:253:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:264:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:264:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:271:38: error: '15' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:303:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:303:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:321:31: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:321:37: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:332:45: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:364:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:369:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:375:19: error: '4' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:378:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:379:19: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:382:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:386:58: error: '249' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:386:63: error: '172' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:386:68: error: '60' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:387:60: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:387:63: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:387:66: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:387:69: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:408:19: error: '5' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/MainWindow.java:413:23: error: '3' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:66:58: error: '12' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:67:61: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:67:64: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:67:67: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:67:70: error: '8' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:71:56: error: '10' is a magic number.

/var/tmp/upm-swing/src/com/_17od/upm/gui/AboutDialog.java:83:56: error: '10' is a magic number.

Audit done.

Checkstyle ends with 223 errors.

/var/tmp $

Do user need to fix all of this ?

If you do think , please do fork and refactoring and show results for

https://github.com/adrian/upm-swing/blob/master/src/com/_17od/upm/gui/MainWindow.java

I might be not aware of some good patters of GUI code, please share an experience.

After refactoring please create PR to this project and try to persuade author to merge.

Michał Kordas

unread,

Oct 2, 2015, 1:52:50 PM10/2/15

to checkstyle-devel

Hi,

First of all I would allow for GUI code numbers passed as method arguments when the result is assigned to constant.

From https://github.com/adrian/upm-swing/blob/master/src/com/_17od/upm/gui/MainWindow.java#L386

databaseFileChangedPanel.setBackground(new Color(249, 172, 60)); //not helpful,as reader has no idea what color it is

private static final Color ORANGE = new Color(249, 172, 60); //OK

Then there are cases like https://github.com/adrian/upm-swing/blob/master/src/com/_17od/upm/gui/AboutDialog.java#L66 - I don't think that it should be hardcoded so deeply, it should be rather some common constant.

Duplication like in https://github.com/adrian/upm-swing/search?utf8=%E2%9C%93&q=DATABASE_AUTO_LOCK_TIME - code Preferences.getInt(Preferences.ApplicationOptions.DATABASE_AUTO_LOCK_TIME, 5) is used twice. Reader has no idea what 5 means here. Moreover, when this "5" changes in one place, the second place most probably should also be updated.

Constructor of class https://github.com/adrian/upm-swing/blob/master/src/com/_17od/upm/gui/AccountDialog.java#L117 has 600 lines of code. Someone who writes so long constructors, probably don't care about Checkstyle anyway. Instead of duplication and putting numbers everywhere, there should be some algorithm that generates the layout based on the available window size.

If someone has very specific needs and needs to hardcode many numbers to display things properly because of pixel-by-pixel handcrafted GUI, I don't see an issue to suppress magic numbers on some particular classes like ending with *Dialog or *Window. But saying that magic numbers do not apply to GUI code is too much. GUI code is like any other code. If you need to do special things with GUI then suppress. But also suppress if you e.g. do special things with maths and deliberately decide that putting magic numbers everywhere improves readability.

Thanks,

Michal

Roman Ivanov

unread,

Oct 6, 2015, 2:23:31 AM10/6/15

to Michał Kordas, checkstyle-devel

Hi Michal,

> First of all I would allow for GUI code numbers passed as method arguments when the result is assigned to constant.

at least you agree that UI code should have special config for MagicNumber.

> ORANGE = new Color(249, 172, 60); //OK

It is not ok as you are cheating. ORANGE is http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/awt/Color.java#142

so it is some color that user like or suit in this exact place, no name for it probably. RGB for colors is kind of standard, most people know - reg, green, blue.

> I don't think that it should be hardcoded so deeply, it should be rather some common constant.

Do you propose to make variable FONT_SIZE ? I doubt it will be useful, especially it if it is used ones.

Number is non-magic if context is well known, that is why allow user ignoreList to let him deside, what magic and what is not.

> Duplication like in

Reasonable duplication is much better then dependencies from design point of view. World full of duplication and nobody wants to depends on each other. The less dependencies the better (reasonable less!).

> Someone who writes so long constructors, probably don't care about Checkstyle anyway.

Chekcstyle is configurable, if owner is ok with such size, checkstyle will be ok too.

> there should be some algorithm that generates the layout based on the available window size.

in ideal application yes, in most cases code is like in this project, simple and works fine, nobody care.

> *Dialog or *Window. But saying that magic numbers do not apply to GUI code is too much.

yes, I usually mean this: gui - swing code. It is unfortunate when gui code is not singled outed from logic, but this should resolved on design level.

I am not against encourage complete abandoning of MagicNumber in GUI projects, but some packages for sure (or by class name if name notation is enforced)

=================

Violations from Checkstyle:

execute:

[echo] Checkstyle started: 05/10/2015 06:13:01 PM

[checkstyle] Running Checkstyle 6.12-SNAPSHOT on 715 files

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/Main.java:75:27: error: '1500' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/Main.java:75:33: error: '800' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeInfoPanel.java:93:34: error: '20' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeInfoPanel.java:93:38: error: '15' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeModel.java:97:18: error: '3' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeModel.java:100:18: error: '4' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeModel.java:121:18: error: '3' is a magic number.

[checkstyle] checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeModel.java:124:18: error: '4' is a magic number.

ParseTreeModel - reasonable to to update to Enum.

UI swing classes - ParseTreeInfoPanel.java , Main.java . Yes , they are primitive and moving to named variable will not do serious damage to code. But we need to show a principle to community to NOT be obsessed with Checkstyle and use it with common sense in priority ().

I am ok to change suppression to these two files instead of whole package (moving two classes to separate package will be over-engineering).

thanks,

Roman Ivanov

Reply all

Reply to author

Forward

0 new messages