3 Replies Latest reply on Mar 29, 2006 11:38 AM by mazeiar

    JBoss Design Flaws

    mazeiar
    mazeiar Jan 26, 2006 2:51 PM

      Hi,

      I?m a PhD student and recently I?m working on detecting design flaws from the source code. I want to propose a framework, containing a design knowledge-base and a classifier, to automatically detect design flaws in the source code of object-oriented systems. I performed two sets of measurement on JBoss Application Server v. 4.0.0 and now I want to evaluate my work. I really need feedback from designers, developers and experts who know the JBoss design. I?ve selected two design flaws at the class level, namely ?God Class? and ?Shotgun Surgery (see the end of this post). I want to know are these classes really problematic? I mean, it was a design decision to have these god classes and shotgun surgery or it?s a flaw which should be refactored in the future? For instance, if a class has shotgun surgery, for the next releases it may make problems for maintenance and change. So, have these classes made problems in the releases after 4.0.0?

      For your information, I list definitions of these design flaws:

      Design flaw (also known as ?code smell? or ?rotting design?) is any violation of design principles and heuristics. (You can find more info in Martin Fowler?s book on Refactoring)

      ?God class?: A god class is a class that does most of the functionalities of a system in the comparison with other classes in the system. In a nutshell, god Class refers to those classes which tend to centralize the intelligence of the system. An instance of a god class performs most of the operations, delegates only minor details to a set of trivial classes, and uses the data from other classes. This design flaw is partially analogous to Fowler?s Large Class smell. Hence, god classes deviate from the heuristic of manageable complexity, low coupling as well as tend to be also non-cohesive.


      ?Shotgun Surgery?: A class that is coupled to a large number of other classes, and would produce a large number of changes throughout the system in the event of an internal change, contributes to the Shotgun Surgery smell. Shotgun Surgery means that a change in a given class implies many (small) changes to a lot of different classes. By the definition, a class which presents the shotgun surgery flaw tends to be coupled to a large number of other classes. In a nutshell, shotgun surgery deviates from the heuristic of minimization the number of messages in the protocol of the class.

      References:

      M. Fowler, K. Beck, J. Brant, W. Opdyke, and D. Roberts. Refactoring : Improving the Design of Existing Code. Addison-Wesley, 1999.

      A. J. Riel. Object-Oriented Design Heuristics. Addison-Wesley, 1996.


      Measurement #1

      God Class Design Flaw
      Class names
      iiop.CDRStream
      iiop.InterfaceRepository
      jaxrpc.Call
      jaxrpc.SOAPMonitorApplet.SOAPMonitorPage
      jaxrpc.SerializationContextImpl
      server.BasicVisitor
      server.EJBQLToSQL92Compiler
      server.JDBCEJBQLCompiler
      server.JDBCUtil
      server.MarshalledInvocation
      server.ProxyAssembler
      webservice.SOAPMonitorApplet.SOAPMonitorPage
      jmx.org.jboss.mx.capability.OptimizedMBeanDispatcher



      Shotgun Surgery Design Flaw
      Class Names
      cache.Fqn
      cache.TreeCache
      cluster.HAPartition
      connector.JmsMessage
      connector.WrappedStatement
      jaxrpc.AxisFault
      jaxrpc.BasicHandler
      jaxrpc.Constants
      jaxrpc.DeserializationContext
      jaxrpc.EngineConfiguration
      jaxrpc.JavaUtils
      jaxrpc.MessageContext
      jaxrpc.Messages
      jaxrpc.OperationDesc
      jaxrpc.SOAPConstants
      jaxrpc.SOAPElementAxisImpl
      jaxrpc.SerializationContext
      jaxrpc.SymTabEntry
      management.J2EEManagedObject
      management.J2EETypeConstants
      server.AbstractInterceptor
      server.AllowedOperationsAssociation
      server.AllowedOperationsFlags
      server.BeanMetaData
      server.Container
      server.EnterpriseContext
      server.EntityContainer
      server.EntityEnterpriseContext
      server.Interceptor
      server.Invocation
      server.JDBCCMRFieldBridge
      server.JDBCEntityBridge
      server.JDBCEntityMetaData
      server.JDBCFieldBridge
      server.JDBCUtil
      server.MetaData
      server.SQLUtil
      aop.org.jboss.aop.Advisor
      aop.org.jboss.aop.AspectManager
      aop.org.jboss.aop.joinpoint.Invocation
      aop.org.jboss.aop.joinpoint.InvocationBase
      aop.org.jboss.aop.pointcut.MatcherHelper
      aop.org.jboss.aop.pointcut.ast.Node
      aop.org.jboss.aop.pointcut.ast.PointcutExpressionParserVisitor
      aop.org.jboss.aop.pointcut.ast.SimpleNode
      aspects.org.jboss.aspects.versioned.StateManager
      jms.org.jboss.jms.serverless.NotImplementedException
      jmx.javax.management.Descriptor
      jmx.javax.management.MBeanFeatureInfo
      jmx.javax.management.MBeanServer
      jmx.javax.management.MBeanServerConnection
      jmx.javax.management.ObjectName
      jmx.javax.management.RuntimeOperationsException
      jmx.org.jboss.mx.modelmbean.ModelMBeanConstants
      messaging.org.jboss.mq.Connection
      messaging.org.jboss.mq.SpyJMSException
      messaging.org.jboss.mq.SpyMessage
      messaging.org.jboss.mq.SpyMessage.Header
      messaging.org.jboss.mq.il.Invoker
      messaging.org.jboss.mq.il.ServerIL
      messaging.org.jboss.mq.il.uil2.msgs.BaseMsg
      messaging.org.jboss.mq.server.BasicQueue
      messaging.org.jboss.mq.server.JMSServerInterceptor
      messaging.org.jboss.mq.server.JMSServerInterceptorSupport
      system.org.jboss.system.ServiceMBeanSupport




      Measurement #2

      God Class Design Flaw
      Class names
      iiop.CDRStream
      iiop.InterfaceRepository
      jaxrpc.Call
      jaxrpc.SOAPMonitorApplet.SOAPMonitorPage
      jaxrpc.SerializationContextImpl
      server.BasicVisitor
      server.EJBQLToSQL92Compiler
      server.JDBCEJBQLCompiler
      server.JDBCUtil
      server.MarshalledInvocation
      server.ProxyAssembler
      webservice.SOAPMonitorApplet.SOAPMonitorPage
      jmx.org.jboss.mx.capability.OptimizedMBeanDispatcher



      Shotgun Surgery Design Flaw
      Class Names
      AnnotationParserVisitor.java /aop/src/main/org/jboss/aop/annotation/ast
      AspectDefinition.java /aop/src/main/org/jboss/aop/advice
      AspectFactory.java /aop/src/main/org/jboss/aop/advice
      AspectManager.java /aop/src/main/org/jboss/aop
      BaseConnectionManager2.java /connector/src/main/org/jboss/resource/connectionmanager
      ConnectionListener.java /connector/src/main/org/jboss/resource/connectionmanager
      ConstructorMatcher.java /aop/src/main/org/jboss/aop/pointcut
      DistributedState.java /aspects/src/main/org/jboss/aspects/versioned
      DistributedVersionManager.java /aspects/src/main/org/jboss/aspects/versioned
      FamilyClusterInfo.java /cluster/src/main/org/jboss/ha/framework/interfaces
      Instrumentor.java /aop/src/main/org/jboss/aop/instrument
      Interceptor.java /aop/src/main/org/jboss/aop/advice
      Interceptor.java /cache/src/main/org/jboss/cache/interceptors
      Invocation.java /aop/src/main/org/jboss/aop/joinpoint
      InvocationBase.java /aop/src/main/org/jboss/aop/joinpoint
      JMSException.java /j2ee/src/main/javax/jms
      JmsMessage.java /connector/src/main/org/jboss/resource/adapter/jms
      LockStrategy.java /cache/src/main/org/jboss/cache/lock
      MatcherHelper.java /aop/src/main/org/jboss/aop/pointcut
      MetaDataResolver.java /aop/src/main/org/jboss/aop/metadata
      MethodMatcher.java /aop/src/main/org/jboss/aop/pointcut
      MuNumber.java /common/src/main/org/jboss/util
      Mutable.java /common/src/main/org/jboss/util
      NestedThrowable.java /common/src/main/org/jboss/util
      NestedThrowable.java /common/src/main/org/jboss/util
      Node.java /aop/src/main/org/jboss/aop/annotation/ast
      Node.java /aop/src/main/org/jboss/aop/pointcut/ast
      Pointcut.java /aop/src/main/org/jboss/aop/pointcut
      PointcutExpressionParserVisitor.java /aop/src/main/org/jboss/aop/pointcut/ast
      Region.java /cache/src/main/org/jboss/cache/eviction
      SimpleMetaData.java /aop/src/main/org/jboss/aop/metadata
      SimpleNode.java /aop/src/main/org/jboss/aop/pointcut/ast
      SimpleNode.java /aop/src/main/org/jboss/aop/annotation/ast
      StateManager.java /aspects/src/main/org/jboss/aspects/versioned
      SynchronizationManager.java /aspects/src/main/org/jboss/aspects/versioned
      TreeCache.java /cache/src/main/org/jboss/cache
      TreeCacheAop.java /cache/src/main/org/jboss/cache/aop
      TreeCacheAopView.java /cache/src/main/org/jboss/cache/aop
      TreeCacheView.java /cache/src/main/org/jboss/cache
      TreeCacheView2.java /cache/src/main/org/jboss/cache
      TxSupport.java /aspects/src/main/org/jboss/aspects/tx
      Util.java /aop/src/main/org/jboss/aop/pointcut
      VersionManager.java /aspects/src/main/org/jboss/aspects/versioned
      VersionReference.java /aspects/src/main/org/jboss/aspects/versioned
      AbstractInterceptor.java /jmx/src/main/org/jboss/mx/interceptor
      BaseDeserializerFactory.java /jaxrpc/src/main/org/apache/axis/encoding/ser
      BaseSerializerFactory.java /jaxrpc/src/main/org/apache/axis/encoding/ser
      BasicHandler.java /jaxrpc/src/main/org/apache/axis/handlers
      Emitter.java /jaxrpc/src/main/org/apache/axis/wsdl/toJava
      Enum.java /jaxrpc/src/main/org/apache/axis/enum
      Interceptor.java /jmx/src/main/org/jboss/mx/interceptor
      J2EEDeployedObject.java /management/src/main/org/jboss/management/j2ee
      J2EEDomain.java /management/src/main/org/jboss/management/j2ee
      J2EEManagedObject.java /management/src/main/org/jboss/management/j2ee
      J2EETypeConstants.java /management/src/main/org/jboss/management/j2ee
      JavaClassWriter.java /jaxrpc/src/main/org/apache/axis/wsdl/toJava
      JavaWriter.java /jaxrpc/src/main/org/apache/axis/wsdl/toJava
      LoaderRepository.java /jmx/src/main/org/jboss/mx/loading
      ManagedObjectFactory.java /management/src/main/org/jboss/management/j2ee/factory
      MBeanFeatureInfo.java /jmx/src/main/javax/management
      MBeanServerConnection.java /jmx/src/main/javax/management
      Messages.java /jaxrpc/src/main/org/apache/axis/utils
      Monitor.java /jmx/src/main/javax/management/monitor
      NodeImpl.java /jaxrpc/src/main/org/apache/axis/message
      NormalizedString.java /jaxrpc/src/main/org/apache/axis/types
      NotificationConstants.java /management/src/main/org/jboss/management/j2ee
      NotImplementedException.java /jms/src/main/org/jboss/jms/serverless
      OpenType.java /jmx/src/main/javax/management/openmbean
      OperationsException.java /jmx/src/main/javax/management
      RelationException.java /jmx/src/main/javax/management/relation
      RepositoryClassLoader.java /jmx/src/main/org/jboss/mx/loading
      SimpleDeserializer.java /jaxrpc/src/main/org/apache/axis/encoding/ser
      SOAPElementAxisImpl.java /jaxrpc/src/main/org/apache/axis/message
      SOAPElementImpl.java /jaxrpc/src/main/org/apache/axis/message
      SOAPEnvelopeAxisImpl.java /jaxrpc/src/main/org/apache/axis/message
      SOAPFaultImpl.java /jaxrpc/src/main/org/apache/axis/message
      SOAPHandler.java /jaxrpc/src/main/org/apache/axis/message
      StateManagement.java /management/src/main/org/jboss/management/j2ee
      StatisticImpl.java /management/src/main/org/jboss/management/j2ee/statistics
      StatsBase.java /management/src/main/org/jboss/management/j2ee/statistics
      SymTabEntry.java /jaxrpc/src/main/org/apache/axis/wsdl/symbolTable
      tcpmon.java /jaxrpc/src/main/org/apache/axis/utils
      TypeEntry.java /jaxrpc/src/main/org/apache/axis/wsdl/symbolTable
      Utils.java /jaxrpc/src/main/org/apache/axis/wsdl/toJava
      ValueExp.java /jmx/src/main/javax/management
      WSDDConstants.java /jaxrpc/src/main/org/apache/axis/deployment/wsdd
      WSDDDeployableItem.java /jaxrpc/src/main/org/apache/axis/deployment/wsdd
      WSDDDeployment.java /jaxrpc/src/main/org/apache/axis/deployment/wsdd
      WSDDElement.java /jaxrpc/src/main/org/apache/axis/deployment/wsdd
      WSDDHandler.java /jaxrpc/src/main/org/apache/axis/deployment/wsdd
      AbstractDeploymentScanner.java /system/src/main/org/jboss/deployment/scanner
      AbstractInvoker.java /remoting/src/main/org/jboss/remoting
      AbstractServerLoginModule.java /security/src/main/org/jboss/security/auth/spi
      BaseMsg.java /messaging/src/main/org/jboss/mq/il/uil2/msgs
      BasicQueue.java /messaging/src/main/org/jboss/mq/server
      Connection.java /messaging/src/main/org/jboss/mq
      DeploymentException.java /system/src/main/org/jboss/deployment
      DeploymentInfo.java /system/src/main/org/jboss/deployment
      InvocationRequest.java /remoting/src/main/org/jboss/remoting
      JMSDestinationManager.java /messaging/src/main/org/jboss/mq/server
      JMSServerInterceptor.java /messaging/src/main/org/jboss/mq/server
      MessagePool.java /messaging/src/main/org/jboss/mq
      MessageReference.java /messaging/src/main/org/jboss/mq/server
      MsgTypes.java /messaging/src/main/org/jboss/mq/il/uil2/msgs
      OIL2Request.java /messaging/src/main/org/jboss/mq/il/oil2
      OIL2SocketHandler.java /messaging/src/main/org/jboss/mq/il/oil2
      SocketManager.java /messaging/src/main/org/jboss/mq/il/uil2
      SpyJMSException.java /messaging/src/main/org/jboss/mq
      SpyMessage.java /messaging/src/main/org/jboss/mq
      SpyMessage.java /messaging/src/main/org/jboss/mq
      SpySession.java /messaging/src/main/org/jboss/mq
      SubDeployer.java /system/src/main/org/jboss/deployment
      Subscription.java /messaging/src/main/org/jboss/mq
      UsernamePasswordLoginModule.java /security/src/main/org/jboss/security/auth/spi



      • 15075 Views
      • Tags:
        • 1. Re: JBoss Design Flaws
          acoliver
          acoliver Feb 21, 2006 10:09 PM (in response to mazeiar)

          Some of these I laugh and say "haha...you got it". Others its like "the spec this came from makes it that way or its a simplification" and some are like "who cares?" ... Particularly funny is so much of JBossMQ (which we're rewriting) and TreeCache (you should open the file).

          However, I might suggest a different approach than listing these and just asking the developers in a forum. One no one is probably as interested in your research as you are (objective mesurement for a subjective issue) and there are social aspects that make honest replies likely to be spartan and controversial. Moreover, what is in it for us really? Consider offering to fix one or two or getting involved such that you say "I think XYZ should be redesigned" and why. Then based on the discussion and reaction you your patches you'll get concrete information.. However you just asked busy people interested in other things to do your work for you... Good book from harvard uni press: http://www.amazon.com/gp/product/0674012925/002-6943602-5605610?v=glance&n=283155

            • Actions
          • 2. Re: JBoss Design Flaws
            adrian.brock Feb 22, 2006 6:39 AM (in response to mazeiar)

             

            "mazeiar" wrote:

            ?Shotgun Surgery?: A class that is coupled to a large number of other classes


            I think there is something wrong with your analysis when you cannot differentiate
            a contract (spec or otherwise) from this "design flaw".

            Contracts don't change.

            I do agree on some of the JBossMQ implementation, e.g. BasicQueue.
            In fact, you can find my complaints about this class all over the forums. ;-)

            Strangely, you didn't pick on a worse class
            org.jboss.ejb.plugins.jms.JMSContainerInvoker
            I don't know how many design flaws are involved and its related classes, but it is lots. ;-)
            That doesn't mean it doesn't work, it just means it is hard to change.

              • Actions
            • 3. Re: JBoss Design Flaws
              mazeiar
              mazeiar Mar 29, 2006 11:38 AM (in response to mazeiar)


              The analysis is only based on the source code, so I agree with you that it cannot discriminate design decisions/contracts from design flaws. Actually, this is why I post this message to get feedback from developers. I'm thinking about using other design artifacts to help classifiers but as you know these information are still rarely structured and machine-readable. For the case of model-driven approaches, UML-based design documents and system architecture can be useful, but for JBoss I don't have access to these documents.

              In fact, I started my work by architectural recovery to extract the concrete architecture, as implemented, and find the differences (deviations) with abstract architecture, as designed. But I was looking for an objective measurement for finding design flaws and this was the main casue of trying to analyze the source code by well-known OO metrics.

              I use a two-level filter (classifier) to detect design flaws, in this case only two flaws. For org.jboss.ejb.plugins.jms.JMSContainerInvoker my measurements show it has been detected by the first level filter as a hotspot and potential flaw (due to highly coupled with other classes) but it's been rejected by the second filter (for shotgun surgery). The reason is that for this flaw only coupling with other classes is not enough, but the fan-in is very important. It means how much the other classes depend on this class which may lead to maintenance/evolution problems in the future in case of change in this class.

              Thank you for your reply.


              "adrian@jboss.org" wrote:
              "mazeiar" wrote:

              ?Shotgun Surgery?: A class that is coupled to a large number of other classes


              I think there is something wrong with your analysis when you cannot differentiate
              a contract (spec or otherwise) from this "design flaw".

              Contracts don't change.

              I do agree on some of the JBossMQ implementation, e.g. BasicQueue.
              In fact, you can find my complaints about this class all over the forums. ;-)

              Strangely, you didn't pick on a worse class
              org.jboss.ejb.plugins.jms.JMSContainerInvoker
              I don't know how many design flaws are involved and its related classes, but it is lots. ;-)
              That doesn't mean it doesn't work, it just means it is hard to change.


                • Actions