experiences reviewing scientific c++ code marc paterno f cd/special assignments

18
Experiences Reviewing Experiences Reviewing Scientific C++ Code Scientific C++ Code Marc Paterno Marc Paterno f f CD/Special CD/Special Assignments Assignments

Upload: warren-booth

Post on 28-Dec-2015

215 views

Category:

Documents


3 download

TRANSCRIPT

Page 1: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Experiences Reviewing Experiences Reviewing Scientific C++ CodeScientific C++ Code

Marc PaternoMarc Paternoff CD/Special Assignments CD/Special Assignments

Page 2: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

OutlineOutline

BackgroundBackground AbstractionAbstraction

Need, use and misuseNeed, use and misuse LibrariesLibraries

Standard libraryStandard library HEP librariesHEP libraries

Page 3: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Most useful feature of C++Most useful feature of C++

Clearly, the most Clearly, the most usefuluseful::support for support for abstractionabstraction, both in OO , both in OO design and generic programmingdesign and generic programming

To appreciate this, let’s have a look at To appreciate this, let’s have a look at where we’re coming from… minimally where we’re coming from… minimally structured FORTRAN.structured FORTRAN.

Page 4: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

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

IPVT(K)=M IF(M.NE.K)IPVT(N)=-IPVT(N) T=A(M,K) A(M,K)=A(K,K) A(K,K)=TC IF(T.EQ.0.0)GOTO 35C DO 20 I=KP1,N A(I,K)=-A(I,K)/T 20 CONTINUEC 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

Page 5: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

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

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

Page 6: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

The most problematical The most problematical feature…feature…

Unfortunately the most Unfortunately the most problematicalproblematical feature of C++ has also been feature of C++ has also been abstraction, generally in the context of abstraction, generally in the context of OO design.OO design.

It is not always easy to determine the It is not always easy to determine the right level of abstraction for a given use. right level of abstraction for a given use. For many physicists, previous For many physicists, previous programming experience doesn’t apply programming experience doesn’t apply -- new techniques have to be learned.-- new techniques have to be learned.

Page 7: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Common mistakes with Common mistakes with abstractionabstraction

MissingMissing the useful abstraction the useful abstraction Classes used as Classes used as structsstructs, if used at all, if used at all Procedural code, where OO would be betterProcedural code, where OO would be better

Too manyToo many abstractions abstractions Many levels of inheritanceMany levels of inheritance Base classes never usedBase classes never used Often, casting requiredOften, casting required

Classes that are Classes that are too largetoo large Too many tasks performed by a single classToo many tasks performed by a single class Resulting classes are difficult to understand Resulting classes are difficult to understand

and useand use

Page 8: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Missing the AbstractionMissing the Abstraction

Common Mistake #1: Common Mistake #1: missing classmissing class Set of functions pass around a common set Set of functions pass around a common set

of arguments, or perhaps an arrayof arguments, or perhaps an array Users have to keep track of the arguments Users have to keep track of the arguments

themselvesthemselves Common Mistake #2: Common Mistake #2: do-nothing classdo-nothing class

Contains set & get methodsContains set & get methods Users extract values, perform calculations Users extract values, perform calculations

themselvesthemselves Often not hard to solve: Often not hard to solve: refactoringrefactoring

Page 9: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

XList*Alg::createXList(const Hit& hit1, const Hit& hit2) {XList* list = new XList;double x1 = hit1.pos().pos().x();double y1 = hit1.pos().pos().y();double z1 = hit1.pos().pos().z();double d2 = hit1.hit().drift();double x2 = hit2.pos().pos().x();double y2 = hit2.pos().pos().y();double z2 = hit2.pos().pos().z();double d2 = hit2.hit().drift();Point p11( x1, y1-d1, z1 );Point p12( x1, y1+d1, z1 );Point p21( x2, y2-d2, z2 );Point p22( x2, y2+d2, z2 );list->push_back(new X(p11, p21));list->push_back(new X(p11, p22));list->push_back(new X(p12, p21));list->push_back(new X(p12, p22));return list;

}

Missing class functionalityMissing class functionality

Page 10: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

XList*Alg::createXList(const Hit& hit1, const Hit& hit2) { List* li = new XList;li->push_back(new X(hit1.negPoint(), hit2.negPoint()));li->push_back(new X(hit1.negPoint(), hit2.posPoint()));li->push_back(new X(hit1.posPoint(), hit2.negPoint()));li->push_back(new X(hit1.posPoint(), hit2.posPoint()));return li;

}

RefactoringRefactoring

Exception safety is also a common problem

Page 11: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Too many abstractionsToo many abstractions

Common Mistake #3: Common Mistake #3: unused base classunused base class Abstract base class introduced, but only one Abstract base class introduced, but only one

subclass is ever producedsubclass is ever produced Cost both in program efficiency and Cost both in program efficiency and

maintenancemaintenance Common Mistake #4: Common Mistake #4: very deep very deep

hierarchieshierarchies Add a new layer of inheritance for one or two Add a new layer of inheritance for one or two

new functions; no users are interested in the new functions; no users are interested in the middle layers of the hierarchymiddle layers of the hierarchy

Much more difficult for users to understand the Much more difficult for users to understand the codecode

Page 12: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Needless AbstractionNeedless Abstraction

Only one subclass of Only one subclass of each abstract class each abstract class existsexists

Users who get an Users who get an AbsComponent*AbsComponent* from the from the AbsThingAbsThing interface have to interface have to (dynamic) cast (dynamic) cast before they can use before they can use it to get at function it to get at function h()h()..

+f() : AbsComponent

AbsThing

+g() : void

AbsComponent

+h() : void

ComponentThing

Needless complexity makes code harder to understand.

Page 13: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Classes that do too muchClasses that do too much

FatFat interface interface Scores of unrelated functionsScores of unrelated functions Often, functions form several related Often, functions form several related

groupingsgroupings MultipleMultiple purposes purposes

Member datum that says if we are to Member datum that says if we are to perform task perform task AA or task or task BB

… … even worse if we have to query this state even worse if we have to query this state from outside the classfrom outside the class

Page 14: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

A fat interface exampleA fat interface example

““The [BaseObject] class provides default The [BaseObject] class provides default behavior and protocol for all objects … It behavior and protocol for all objects … It provides protocol for provides protocol for object I/Oobject I/O, , error error handlinghandling, , sortingsorting, , inspectioninspection, , printingprinting, , drawingdrawing, etc. Every object which inherits from , etc. Every object which inherits from [BaseObject] can be stored in the … [BaseObject] can be stored in the … collection classes.”collection classes.”

What happens when some subclass can’t What happens when some subclass can’t sensibly implement the entire interface? sensibly implement the entire interface? We get functions that don’t make sense.We get functions that don’t make sense.

Page 15: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

Abstraction: summaryAbstraction: summary

It seems to be quite difficult to find just It seems to be quite difficult to find just the right level of abstraction. Cases of the right level of abstraction. Cases of too little abstraction seem easier to too little abstraction seem easier to repair than cases of too much repair than cases of too much abstraction. This may be partially abstraction. This may be partially sociological.sociological.

Wrong abstraction makes code that is Wrong abstraction makes code that is difficult to understand -- and difficult to difficult to understand -- and difficult to (re)use.(re)use.

Page 16: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

We need… a few good librariesWe need… a few good libraries

Dearth of high-quality scientific C++ Dearth of high-quality scientific C++ libraries.libraries. Many projects were started with Many projects were started with early versionsearly versions of of

C++.C++. It has been difficult for these to catch up with It has been difficult for these to catch up with

modernmodern C++: C++: Home-grown string, collection classesHome-grown string, collection classes No use of exceptions -- query No use of exceptions -- query isGood()isGood() to check objects to check objects Lack of exception safetyLack of exception safety No use of templatesNo use of templates

The result is often not The result is often not robustrobust, less , less efficientefficient than than possible, and more possible, and more difficult to maintaindifficult to maintain than than need be.need be.

Page 17: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

How is the Standard Library How is the Standard Library Used?Used?

ContainersContainers are used frequently are used frequently Except where use of some other library’s Except where use of some other library’s

collection(s) have taken over!collection(s) have taken over! New users have some trouble choosing the New users have some trouble choosing the

right container from the Standard Libraryright container from the Standard Library IteratorsIterators are used simplistically are used simplistically

Commonly used in loopsCommonly used in loops Rarely see a pair of iterators used as a rangeRarely see a pair of iterators used as a range

AlgorithmsAlgorithms are chronically under-used are chronically under-used Rarer still are user-invented generic Rarer still are user-invented generic

algorithmsalgorithms

Page 18: Experiences Reviewing Scientific C++ Code Marc Paterno f CD/Special Assignments

SummarySummary

We have made vast improvement over We have made vast improvement over barely-structured Fortran.barely-structured Fortran.

Making good use of abstraction is Making good use of abstraction is difficult.difficult.

We don’t take sufficient advantage of We don’t take sufficient advantage of the Standard Library.the Standard Library.

In short, we see too little use of In short, we see too little use of modernmodern C++.C++.