3
\$\begingroup\$

I have converted this code from Java to Scala, and I need reviews. This works fine, but I am sure that something is wrong with this coding pattern.

package models
import org.apache.log4j.ConsoleAppender
import org.apache.log4j.Level
import org.apache.log4j.Logger
import org.apache.log4j.PatternLayout
import org.apache.log4j.RollingFileAppender
import me.prettyprint.cassandra.serializers.StringSerializer
object LogFile extends CassandraConnection {
 val loger = Logger.getRootLogger()
 def exceptionLogs(qName: String, data: String) {
 //data is json String
 val logg = Logger.getLogger(qName + "-" + data)
 loger.setLevel(Level.ERROR)
 val layout = new PatternLayout("[%t] %-5p %c %x - %m %d{ISO8601} %n")
 loger.addAppender(new ConsoleAppender(layout))
 try {
 val fileAppender = new RollingFileAppender(layout, "QLog.txt")
 loger.addAppender(fileAppender)
 logg.error("")
 loger.removeAppender(fileAppender)
 } catch {
 case e: Exception =>
 println(e)
 logg.error("");
 }
 }
}
rolfl
98.1k17 gold badges219 silver badges419 bronze badges
asked Jun 2, 2014 at 5:02
\$\endgroup\$
1
  • \$\begingroup\$ This feels like it's configuring Log4j more than it's doing logging. XML files or a .properties file are meant to be used for configuring Log4j. I would say that there is indeed something wrong with this coding pattern. \$\endgroup\$ Commented Jun 28, 2014 at 20:45

2 Answers 2

5
\$\begingroup\$

I'm not exactly sure what you mean with coding pattern, but there are some things that strike me as odd:

  1. you only log empty messages ... in some of the cases encoding the message in the logger name. This certainly isn't the way log4j is intended to be used.

  2. When ever you call this method basically the same Appender is created added and removed again. This again is a strange way to use log4j. And I'm not even sure what will happen when two threads call this method at the same time.

  3. You are setting the ConsoleAppender and the Pattern over and over again. This at least is superfluous, and might actually be harmful.

  4. The naming is ... suboptimal ... You have two loggers one, is missing a g in its name, the other is abbreviated. None really tells the reader what these loggers are about.

  5. You never ever actually log with loger

So if this doesn't depend on some weird side effect this logs one empty String when it is called and two empty Strings when creating/adding/removing an Appender throws and exception. If this is really what this is supposed to do, it better made it clear in its names and comments.

answered Jun 25, 2014 at 8:04
\$\endgroup\$
1
\$\begingroup\$
  • object LogFile extends CassandraConnection looks very odd.

  • So are loger and logg.

  • layout and fileAppender should probably be static instead of being recreated at every method call.

  • I can't make sense of the code. I don't know log4j that well, but nonetheless, I think I should be able to vaguely understand what is happening. So I can't quite comment on the "coding pattern".

answered Jun 28, 2014 at 14:59
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.