pixie16 / paass

Pixie Acquisition and Analysis Software Suite
https://pixie16.github.io/paassdoc/
GNU General Public License v3.0
10 stars 29 forks source link

The XML files need to have better error handling and XML Parsing needs consolidated #227

Closed spaulaus closed 7 years ago

spaulaus commented 7 years ago

Description of the problem

Right now we have little to no error handling on the utkscan configuration file. We currently have 5 separate files that read the configuration, and three of those five read the same node multiple times. This also breaks a fundamental idea that a class should have just one function. Therefore, we need to remove the XML reading functionality from the classes to help ensure that they do not take on too much responsibility. Here is the breakdown of the current nodes in the configuration and where they are read

Name Read From
Description Globals::Globals
Global Globals::Globals
Reject Globals::Globals
Physical Globals::Globals
Trace Globals::Globals
Cfd Globals::Globals
Fitting Globals::Globals
Map DetectorLibrary::LoadXml
TreeCorrelator DetectorLibrary::LoadXml
DetectorDriver DetectorDriver::LoadDrivers
Map DetectorDriver::ReadCalXml
Map DetectorDriver::ReadWalkXml
Notebook Notebook::Notebook
TreeCorrelator TreeCorrelator::buildTree
TimeCalibrator TimeCalibrator::ReadTimingCalXml

Proposed Solution

I propose that we create an XmlInterface and XmlParser in Resources. This provides us with some base classes that can be used to implement XML reading for any of the programs not just utkscan.

XmlInterface

The XmlInterface will be a singleton that does the following operations:

XmlParser

The XmlParser class provides basic functionality of parsing various nodes. This class also provides some error handling methods that can be used in the children since they will all need some kind of verification.

Cleaning up of the Globals class

The Globals class has historically had a lot of information in it that it did not actually need. For example it had physical constants, type definitions and functions. All of these will be moved to the appropriate places. This class will now at it's core just be a way to access information that is stored in the XML file.

spaulaus commented 7 years ago

What we tested during the review

Removed closing bracket from Configuration node.

Result: (PASS)

The Pugixml library caught the error and the code exited cleanly

Commented out Revision Node

Result : (PASS)

The Error handling on the Globals parsing caught the error and the program exited cleanly.

Commented out Globals node

Result: (PASS)

The code caught the error when checking the root node integrety, and the code exited cleanly.

Turned on Verbosity in the Map node

Result: (PASS)

The map node information was printed to the screen and the information was properly read from the configuration file.

Modified the entires in the map not to see that the changes were reflected in the parsing of the XML.

Result: (PASS)

The changes were reflected.

Added Calibration node in the list of Channel Nodes

Result : (PASS?)

The program simply ignored the extraneous information and parsed everything in the map correctly. Note: Maybe we should add some additional error handling so that users are warned of this? Will create a separate issue to address this.

Removed verbose="true" from Map node

Result : (PASS)

The map information was not printed to the screen.

Removed Time Calibration Node

Result : (PASS)

The program simply continued on without issue.

Changed values in the Vandle node to check that they are read properly.

Result : (PASS)

The program ran without issue and the changes were reflected.

Changed values in the Trace node to check that they are read properly.

Result : (PASS)

The program ran without issue and the changes were reflected.

Changed values in the Vandle node to check that they are read properly.

Result : (PASS)

The program ran without issue and the changes were reflected.

Added a new node to /Configuration

Result : (PASS)

The program ran without issue and the changes were reflected. It told us that it was ignored.

Added a new node to the Map node and it was just ignored.

Result : (PASS)

The program simply continued on without issue.