Caret7 Development Considerations

From Van Essen Lab

(Redirected from Jon Schindler)
Jump to: navigation, search

Contents

Caret7 Development Strategy Comments

  1. We should carefully evaluate the idea of only supporting 64 bit development. In house builds should be both 32 and 64 bit for the time being. We develop software, and in general, places that develop software tend to run more modern hardware than end users. We still have 32 bit machines here, that should tell us something.
  2. While I think that having a build system that is run after check-in should be a priority, I don't think it's necessary in the beginning. Nightly builds should be our target, with automatic builds on check-in as a feature that would be nice to have.
  3. On the minimization of reliance on 3rd party libraries, I'm not sure this goal is capturing what we really want. The goal is to avoid surprises, and minimizing reliance on 3rd party libraries is one strategy to meet this goal. I'm not saying this to be overly precise either. If we make things like reducing dependence on 3rd party libraries a primary goal, then we could end up following this maxim in situations where it makes little sense.
  4. Finally, we should try to make our software work with stock versions of QT and other 3rd party libraries, rather than building our own for each platform.

Coding

  1. I like the idea of putting a bridge between QString and Caret7. This would be an elegant way of reusing QString while still keeping some insulation between Caret7 and QT.

General Guidelines

  1. Concerns should be kept separate. i.e. We shouldn't lump drawing the brain together with window management and data processing together in the same library the way they are with Caret5. Any module or library that has the words "all" or "manages everything" is probably ripe for break-down into smaller modules.
  2. DRY (don't repeat yourself). This principle is both simple and expansive in it's scope of application. Let's start with examples:
    1. Avoid copying and pasting sections of logic or code
    2. Don't repeat parts of names when naming objects (or files). Here's an example from Caret5, (the words BrainModel are redundant, and actually make it harder to distinguish different parts of the program):
BrainModelAlgorithm.cxx
BrainModelAlgorithmException.cxx
BrainModelAlgorithmMultiThreaded.cxx
BrainModelAlgorithmMultiThreadExecutor.cxx
BrainModelAlgorithmRunAsThread.cxx
BrainModelBorderSet.cxx
BrainModelCiftiCorrelationMatrix.cxx
BrainModelCiftiGradient.cxx
BrainModelContours.cxx
BrainModelContourToSurfaceConverter.cxx
BrainModel.cxx
BrainModelIdentification.cxx
BrainModelOpenGL.cxx
BrainModelOpenGLSelectedItem.cxx
BrainModelOpenGLWidget.cxx
BrainModelRunExternalProgram.cxx
BrainModelStandardSurfaceReplacement.cxx
BrainModelSurfaceAffineRegression.cxx
BrainModelSurfaceAndVolume.cxx
BrainModelSurfaceBankStraddling.cxx
BrainModelSurfaceBorderCutter.cxx
BrainModelSurfaceBorderLandmarkIdentification.cxx
BrainModelSurfaceBorderToMetricConverter.cxx
BrainModelSurfaceBorderToPaintConverter.cxx
BrainModelSurfaceCellAttributeAssignment.cxx
BrainModelSurfaceCellDensityToMetric.cxx
BrainModelSurfaceClusterToBorderConverter.cxx
BrainModelSurfaceConnectedSearch.cxx
BrainModelSurfaceConnectedSearchMetric.cxx
BrainModelSurfaceConnectedSearchPaint.cxx
BrainModelSurfaceCurvature.cxx
BrainModelSurfaceCutter.cxx
BrainModelSurface.cxx
BrainModelSurfaceDeformation.cxx
BrainModelSurfaceDeformationFlat.cxx
BrainModelSurfaceDeformationMapCreate.cxx
BrainModelSurfaceDeformationMeasurement.cxx
BrainModelSurfaceDeformationMultiStageSphericalVector.cxx
BrainModelSurfaceDeformationSpherical.cxx
BrainModelSurfaceDeformationSphericalSlits.cxx
BrainModelSurfaceDeformationSphericalVector.cxx
BrainModelSurfaceDeformDataFile.cxx
BrainModelSurfaceDistortion.cxx
BrainModelSurfaceFindExtremum.cxx
BrainModelSurfaceFlatHexagonalSubsample.cxx
BrainModelSurfaceFlattenFullHemisphere.cxx
BrainModelSurfaceFlattenHemisphere.cxx
BrainModelSurfaceFlattenPartialHemisphere.cxx
BrainModelSurfaceFociSearch.cxx
BrainModelSurfaceFociUncertaintyToRgbPaint.cxx
BrainModelSurfaceGeodesic.cxx
BrainModelSurfaceMetricAnovaOneWay.cxx
BrainModelSurfaceMetricAnovaTwoWay.cxx
BrainModelSurfaceMetricClustering.cxx
BrainModelSurfaceMetricCoordinateDifference.cxx
BrainModelSurfaceMetricCorrelationMatrix.cxx
BrainModelSurfaceMetricExtrema.cxx
BrainModelSurfaceMetricFindClustersBase.cxx
BrainModelSurfaceMetricFullWidthHalfMaximum.cxx
BrainModelSurfaceMetricGradient.cxx
BrainModelSurfaceMetricInGroupDifference.cxx
BrainModelSurfaceMetricInterHemClusters.cxx
BrainModelSurfaceMetricKruskalWallisRankTest.cxx
BrainModelSurfaceMetricOneAndPairedTTest.cxx
BrainModelSurfaceMetricSmoothingAll.cxx
BrainModelSurfaceMetricSmoothing.cxx
BrainModelSurfaceMetricTwinComparison.cxx
BrainModelSurfaceMetricTwoSampleTTest.cxx
BrainModelSurfaceMorphing.cxx
BrainModelSurfaceMultiresolutionMorphing.cxx
BrainModelSurfaceNodeColoring.cxx
BrainModelSurfaceOverlay.cxx
BrainModelSurfacePaintAssignRelativeToLine.cxx
BrainModelSurfacePaintSulcalIdentification.cxx
BrainModelSurfacePaintToBorderConverter.cxx
BrainModelSurfacePointLocator.cxx
BrainModelSurfacePointProjector.cxx
BrainModelSurfacePolyhedron.cxx
BrainModelSurfacePolyhedronNew.cxx
BrainModelSurfaceResection.cxx
BrainModelSurfaceROIAssignMetric.cxx
BrainModelSurfaceROIAssignMetricNodeArea.cxx
BrainModelSurfaceROIAssignPaint.cxx
BrainModelSurfaceROIAssignShape.cxx
BrainModelSurfaceROICreateBorderUsingGeodesic.cxx
BrainModelSurfaceROICreateBorderUsingMetricShape.cxx
BrainModelSurfaceROIFoldingMeasurementReport.cxx
BrainModelSurfaceROIIntegratedFoldingIndexReport.cxx
BrainModelSurfaceROIMetricClusterReport.cxx
BrainModelSurfaceROIMetricGradient.cxx
BrainModelSurfaceROIMetricSmoothing.cxx
BrainModelSurfaceROINodeSelection.cxx
BrainModelSurfaceROIOperation.cxx
BrainModelSurfaceROIPaintReport.cxx
BrainModelSurfaceROIProbAtlasOverlapReport.cxx
BrainModelSurfaceROIShapeCorrelationReport.cxx
BrainModelSurfaceROISurfaceXYZMeansReport.cxx
BrainModelSurfaceROITextReport.cxx
BrainModelSurfaceSmoothing.cxx
BrainModelSurfaceSphericalTessellator.cxx
BrainModelSurfaceStandardSphere.cxx
BrainModelSurfaceSulcalDepth.cxx
BrainModelSurfaceSulcalDepthWithNormals.cxx
BrainModelSurfaceSulcalIdentificationProbabilistic.cxx
BrainModelSurfaceTopologyCorrector.cxx
BrainModelSurfaceToVolumeConverter.cxx
BrainModelSurfaceToVolumeSegmentationConverter.cxx
BrainModelVolumeBiasCorrection.cxx
BrainModelVolumeCrossoverHandleFinder.cxx
BrainModelVolume.cxx
BrainModelVolumeFociDensity.cxx
BrainModelVolumeFociUnprojector.cxx
BrainModelVolumeGradient.cxx
BrainModelVolumeHandleFinder.cxx
BrainModelVolumeLigaseSegmentation.cxx
BrainModelVolumeNearToPlane.cxx
BrainModelVolumeProbAtlasToFunctional.cxx
BrainModelVolumeRegionOfInterest.cxx
BrainModelVolumeROIAtlasResamplingAndSmoothing.cxx
BrainModelVolumeROIGradient.cxx
BrainModelVolumeROIMinima.cxx
BrainModelVolumeROISmoothing.cxx
BrainModelVolumeSegmentationStereotaxic.cxx
BrainModelVolumeSureFitErrorCorrection.cxx
BrainModelVolumeSureFitSegmentation.cxx
BrainModelVolumeTFCE.cxx
BrainModelVolumeThresholdSegmentation.cxx
BrainModelVolumeTopologicalError.cxx
BrainModelVolumeTopologyGraphCorrector.cxx
BrainModelVolumeTopologyGraph.cxx
BrainModelVolumeToSurfaceConverter.cxx
BrainModelVolumeToSurfaceMapperAlgorithmParameters.cxx
BrainModelVolumeToSurfaceMapper.cxx
BrainModelVolumeToSurfaceMapperPALS.cxx
BrainModelVolumeToVtkSurfaceMapper.cxx
BrainModelVolumeVoxelColoring.cxx

3 Dependencies should travel in one direction only, except under the most unusual circumstances. i.e. The library responsible for reading NIFTI can rely on lower level file reader/writers, but in no circumstances should low-level tools "know" anything about the stuff that relies on them. Allowing modules to be interdependent makes it nearly impossible to rewrite software.

Variable and Function Naming

  1. Object, function, and variable names should accurately describe what is going on. We shouldn't just come up with a name for something and hope people get used to it. Misleading and inaccurate names are never acceptable and can be a huge time-sink.
  2. Object names should be nouns. Function and caret_command names should be verbs. Variable names should be nouns.

Spend some time thinking about the name of your variables and functions. Avoid the temptation to use names like "temp", "x", or other meaningless variable names. While it may make sense to you while you are writing it, chances are you won't remember what you did 6 months later, much less the poor maintenance programmer who has to figure out what you did.

At the other end of the spectrum, avoid coming up with function names like BrainComputationModelWithColorFunctionCalculationPrintTwice. There are several things wrong with very long function names: 1. They indicate you are trying to do too much in one block of code, which means you aren't writing modular code. 2. Most long function names aren't verbs, which is very confusing. (think about it, a function applies some kind of transformation to a piece of data, so it should almost always be a verb) 3. Camel case is difficult to read, and when people are skimming through your code, they may mistinterpret what your very long function name is actually doing by focusing on the wrong part.

While reflexively using long names seems like a cure-all for short names, the real cure is to put some thought into it. Avoid function names that are nouns, they hardly ever describe what you mean. The majority of function names should be verbs, preferably in the imperative tone, i.e.:

good - CalculateCoordinates

worse - getBrainModelCoordinates (using get when you mean calculate or vice versa, is a bad habit)

bad - BrainModelCoordinates (this is a noun, and more appropriate for a variable name)

Contrary to the above

Avoid hard and fast rules. Sometimes it makes sense to use a single line if statement, for example, when checking for errors: if(error) throw FileException("Error opening file.");

In areas where you know there is only one line, it usually doesn't make sense to bracket blocks.

Bad Smells in Code

To paraphrase from the article below, a "smell" in code is a sign or symptom in source code of an underlying design flaw.
http://en.wikipedia.org/wiki/Code_smell
http://en.wikipedia.org/wiki/Anti-pattern

From the example list given above: Duplicated code: identical or very similar code exists in more than one location.

  • This is quite common in caret5, most notably in caret_command, with quite a bit of repetition in argument parsing.

Long method: a method, function, or procedure that has grown too large.

  • This is also common in caret5

Large class: a class that has grown too large. See God object.

  • In caret5 several objects use the word "all" or "manages everything to do with..." to describe their behavior. There is very rarely a good reason to throw everything into a single object.

Feature envy: a class that uses methods of another class excessively. Inappropriate intimacy: a class that has dependencies on implementation details of another class.

  • This was especially common in caret_files.

Refused bequest: a class that overrides a method of a base class in such a way that the contract of the base class is not honored by the derived class. See Liskov substitution principle.

  • Incorrect usage of object oriented design was perhaps one of the biggest issues with Caret5. There were many cases of derived classes that broke the contract that was implied by the base class. There were also many examples of base classes that really did not function as base classes.

Lazy class / Freeloader: a class that does too little. Contrived complexity: forced usage of overly complicated design patterns where simpler design would suffice.

  • This happened quite often in caret5, where the desire to fit everything into an inheritance model of software organization resulted in unneeded complexity.

Excessively long identifiers: in particular, the use of naming conventions to provide disambiguation that should be implicit in the software architecture. Excessive use of literals: these should be coded as named constants, to improve readability and to avoid programming errors. Additionally, literals can and should be externalized into resource files/scripts where possible, to facilitate localization of software if it is intended to be deployed in different regions.


Some others that aren't mentioned are:

  1. Deeply nested logic. If you find that 80 columns isn't quite enough, or you want to reduce your indentation to 2 or 3 characters, it's a sign that you should be breaking your logic down into subroutines. You can always use inline function calls if speed is a concern.
  2. Lots of comments. Sometimes too many comments can actually be a sign that you aren't naming your variables and functions properly, and that you aren't breaking your code down into clearly defined logical entities. A very long sub-routine with every line commented is usually begging to be refactored into a list of functions with sensible names.
Personal tools
Sums Database