Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java https://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java https://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety https://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: https://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
https://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: https://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
http://programmers.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530 https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
http://programmers.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

Source Link
ASA
  • 463
  • 2
  • 9

1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!: http://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
http://programmers.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530

2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.

3) Properly build paths
Use the File constructor to combine path parts:
http://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log") and then calling file.getParentFile().mkdirs(); would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.

4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"

5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting: http://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.

6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.

7) Append cascade
Is there any advantage to calling write(message.getBytes()); instead of println/print (message)? I have never seen that before.

8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.

9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.

lang-java

AltStyle によって変換されたページ (->オリジナル) /