# Presentation: 293544

## Experiences Reviewing Scientific C++ Code

- Marc Paternof CD/Special Assignments

## Outline

- Background
- Abstraction
- Need, use and misuse
- Libraries
- Standard library
- HEP libraries

## Review experience

- J. Kowalkowski & I are part of the CD/SA department.
- One of our major tasks is reviewing C++ designs and implementations for experiments.
- 7 major reviews in last 18 months
- Analysis objects		Track reconstruction (x2)
- Calorimeter reconstruction	Muon reconstruction
- Detector simulation		(Un)packing of raw data
- Most reviews are of significant subsystems
- 3-10 packages	 (libraries)	50+ classes

## Most useful feature of C++

- Clearly, the most useful:support for abstraction, both in OO design and generic programming
*	**To appreciate this, let’s have a look at where we’re coming from... minimally structured FORTRAN.*

##  SUBROUTINE DECOMP(NDIM,N,A,COND,IPVT,WORK)
      IMPLICIT DOUBLE PRECISION (A-H,O-Z)
      DIMENSION A(NDIM,N),WORK(N),IPVT(N)
      IPVT(N)=1
      IF(N.EQ.1)GOTO 80
      NM1=N-1
      ANORM=0.0
      DO 10 J=1,N
      T=0.0
      DO 5 I=1,N
      T=T+DABS(A(I,J))
    5 CONTINUE
      IF(T.GT.ANORM)ANORM=T
   10 CONTINUE
      DO 35 K=1,NM1
      KP1=K+1
      M=K
      DO 15 I=KP1,N
      IF(DABS(A(I,K)).GT.DABS(A(M,K)))M=I
   15 CONTINUE

**      ****IMPLICIT DOUBLE PRECISION (A-H,O-Z)**
**      ****DIMENSION A(NDIM,N),WORK(N),IPVT(N)**
**      ****IPVT(N)=1**
**      ****IF(N.EQ.1)GOTO 80**
**      ****NM1=N-1**
**      ****ANORM=0.0**
**      ****DO 10 J=1,N**
**      ****T=0.0**
**      ****DO 5 I=1,N**
**      ****T=T+DABS(A(I,J))**
**    ****5 CONTINUE**
**      ****IF(T.GT.ANORM)ANORM=T**
**   ****10 CONTINUE**
**      ****DO 35 K=1,NM1**
**      ****KP1=K+1**
**      ****M=K**
**      ****DO 15 I=KP1,N**
**      ****IF(DABS(A(I,K)).GT.DABS(A(M,K)))M=I**
**   ****15 CONTINUE**
**      **

**IPVT(K)=M**
**      ****IF(M.NE.K)IPVT(N)=-IPVT(N)**
**      ****T=A(M,K)**
**      ****A(M,K)=A(K,K)**
**      ****A(K,K)=T**
**C**
**      ****IF(T.EQ.0.0)GOTO 35**
**C**
**      ****DO 20 I=KP1,N**
**      ****A(I,K)=-A(I,K)/T**
**   ****20 CONTINUE**
**C**
**      ****DO 30 J=KP1,N**
**      ****T=A(M,J)**
**      ****A(M,J)=A(K,J)**
**      ****A(K,J)=T**
**      ****IF(T.EQ.0.0)GOTO 30**
**      ****DO 25 I=KP1,N**
**      ****A(I,J)=A(I,J)+A(I,K)*T**
**   ****25 CONTINUE**
**   ****30 CONTINUE**
**   ****35 CONTINUE**

##       DO 50 K=1,N
      T=0.0
      IF(K.EQ.1)GOTO 45
      KM1=K-1
      DO 40 I=1,KM1
      T=T+A(I,K)*WORK(I)
   40 CONTINUE
   45 EK=1.0
      IF(T.LT.0.0)EK=-1.0
      IF(A(K,K).EQ.0.0)GOTO 90
      WORK(K)=-(EK+T)/A(K,K)
   50 CONTINUE
      DO 60 KB=1,NM1
      K=N-KB
      T=0.0
      KP1=K+1
      DO 55 I=KP1,N
      T=T+A(I,K)*WORK(K)
   55 CONTINUE
      WORK(K)=T
      M=IPVT(K)
      IF(M.EQ.K)GOTO 60
      T=WORK(M)
      WORK(M)=WORK(K)
      WORK(K)=T
   60 CONTINUE

**      ****T=0.0**
**      ****IF(K.EQ.1)GOTO 45**
**      ****KM1=K-1**
**      ****DO 40 I=1,KM1**
**      ****T=T+A(I,K)*WORK(I)**
**   ****40 CONTINUE**
**   ****45 EK=1.0**
**      ****IF(T.LT.0.0)EK=-1.0**
**      ****IF(A(K,K).EQ.0.0)GOTO 90**
**      ****WORK(K)=-(EK+T)/A(K,K)**
**   ****50 CONTINUE**
**      ****DO 60 KB=1,NM1**
**      ****K=N-KB**
**      ****T=0.0**
**      ****KP1=K+1**
**      ****DO 55 I=KP1,N**
**      ****T=T+A(I,K)*WORK(K)**
**   ****55 CONTINUE**
**      ****WORK(K)=T**
**      ****M=IPVT(K)**
**      ****IF(M.EQ.K)GOTO 60**
**      ****T=WORK(M)**
**      ****WORK(M)=WORK(K)**
**      ****WORK(K)=T**
**   ****60 CONTINUE**

**      ****YNORM=0.0**
**      ****DO 65 I=1,N**
**      ****YNORM=YNORM+DABS(WORK(I))**
**   ****65 CONTINUE**
**      ****CALL SOLVER(NDIM,N,A,****              WORK,IPVT)****   ZNORM=0.0**
**      ****DO 70 I=1,N**
**      ****ZNORM=ZNORM+DABS(WORK(I))**
**   ****70 CONTINUE**
**      ****COND=ANORM*ZNORM/YNORM**
**      ****IF(COND.LT.1.0)COND=1.0**
**      ****RETURN**
**   ****80 COND=1.0**
**      ****IF(A(1,1).NE.0.0)RETURN**
**   ****90 COND=1.0E32**
**      ****RETURN**
**      ****END**

## The most problematical feature...

- Unfortunately the most problematical feature of C++ has also been abstraction, generally in the context of OO design.
*	**It is not always easy to determine the right level of abstraction for a given use. For many physicists, previous programming experience doesn’t apply -- new techniques have to be learned.*

## Common mistakes with abstraction

- Missing the useful abstraction
- Classes used as *structs*, if used at all
- Procedural code, where OO would be better
- Too many abstractions
- Many levels of inheritance
- Base classes never used
- Often, casting required
- Classes that are too large
- Too many tasks performed by a single class
- Resulting classes are difficult to understand and use*	*

## Missing the Abstraction

- Common Mistake #1: missing class
- Set of functions pass around a common set of arguments, or perhaps an array
- Users have to keep track of the arguments themselves
- Common Mistake #2: do-nothing class
- Contains set & get methods
- Users extract values, perform calculations themselves
- Often not hard to solve: refactoring

## Missing class functionality

**}**
**    **

## Refactoring

**}**

*Exception safety is also a common problem*

## Typical review comment

*	**...**we had a difficult time telling what exactly the algorithm was and what it was doing.  One of our concerns is that we could not tell what manipulations are completely defined by the detector design (fixed angle calculation, fixed relationships of components) and which were actually algorithmic... *
*	**The source of this problem is that the algorithm has not been decomposed properly into manageable units.  Many of the tasks are performed directly by the algorithm code, as opposed to utilities and tools.  The tasks perform operations on dumb data structures rather then real objects with behavior.** *

## Too many abstractions

- Common Mistake #3: unused base class
- Abstract base class introduced, but only one subclass is ever produced
- Cost both in program efficiency and maintenance
- Common Mistake #4: very deep hierarchies
- Add a new layer of inheritance for one or two new functions; no users are interested in the middle layers of the hierarchy
- Much more difficult for users to understand the code

## Needless Abstraction

- Only one subclass of each abstract class exists
- Users who get an *AbsComponent** from the *AbsThing* interface have to (dynamic) cast before they can use it to get at function *h()*.

*Needless complexity makes code harder to understand.*

## Classes that do too much

- Fat interface
- Scores of unrelated functions
- Often, functions form several related groupings
- Multiple purposes
- Member datum that says if we are to perform task *A* or task *B*
- ... even worse if we have to query this state from outside the class

## A fat interface example

- 	“*The  [BaseObject] class provides default behavior and protocol for all objects ... It provides protocol for **object I/O**, **error handling**, **sorting**, **inspection**, **printing**, **drawing**, etc. Every object which inherits from [BaseObject] can be stored in the ... collection classes.”*
- What happens when some subclass can’t sensibly implement the entire interface? We get functions that don’t make sense.

## Abstraction: summary

*	**It seems to be quite difficult to find just the right level of abstraction. Cases of too little abstraction seem easier to repair than cases of too much abstraction. This may be partially sociological.*
*	**Wrong abstraction makes code that is difficult to understand -- and difficult to (re)use.*

## We need... a few good libraries

- Dearth of high-quality scientific C++ libraries.
- Many projects were started with early versions of C++.
- It has been difficult for these to catch up with modern C++:
- Home-grown string, collection classes
- No use of exceptions -- query *isGood()* to check objects
- Lack of exception safety
- No use of templates
- The result is often not robust, less efficient than possible, and more difficult to maintain than need be.

## How is the Standard Library Used?

- Containers are used frequently
- Except where use of some other library’s collection(s) have taken over!
- New users have some trouble choosing the right container from the Standard Library
- Iterators are used simplistically
- Commonly used in loops
- Rarely see a pair of iterators used as a range
- Algorithms are chronically under-used
- Rarer still are user-invented generic algorithms

## Summary

- We have made vast improvement over barely-structured Fortran.
- Making good use of abstraction is difficult.
- We don’t take sufficient advantage of the Standard Library.
*In short, we see too little use of **modern** C++.*