Friday, July 21, 2006
Code Review: Code name Bellagio
Overview
BELLAGIO EMR (Electronic Medical Record) is a custom solution targeting healthcare providers. Its purpose is to automate common record keeping and retrieval, document management and scheduling. Medical records are patient centric and it uses windows form as a front end.
The core modules of BELLAGIO EMR include patient registration and encounter management module, task/scheduling module and a document module. Others are Form Builder/BELLAGIO EMR survey, EMR Reporting and a couple of other modules are in development.
Architecture
BELLAGIO EMR is built primarily with c#.Net. SqlServer database serves as its backend. Other technologies in use include Sql server Reporting services, Office Automation, Office 2003 Reporting, Xml. Tools and platforms include VS 2003 and Infragistics toolset for the presentation layer.
Authentication and authorization is done with Active directory.
The application has two physical layer: A heavy client and a database server. Logically the partitions include a Windows form UI project, a BusinessFacade project that implements the façade pattern. The BusinessRules class library defines CRUD functions and some entity specific operations, mostly making database calls via an object persistence framework OPF. The ObjectPersistenceFramework (OPF) is an in-house library that abstracts ADO.Net classes and provides method calls like Create(), Update etc. There is a BusinessEntities project that is just a handful of transport objects with public properties for setting and getting member variables. It does some little manipulations on strings, dates and numbers which are sometimes not persisted to the database..
Other class libraries included in the solution is a Utility project containing enumerations, some date and string manipulation functions and some logging functions. The SpCore class library abstract reusable codes and is not directly tied to EMR.
The only visible pattern we identified in EMR is the Façade pattern.
We did identify the solution would could have benefited from an MVC pattern. An Observer pattern using events and delegates would have been ideal for updating the multiple views in EMR, an approach that would make the application more scalable and reduced maintenance headaches. This could also provide a framework for automation and plug-ins
.
We identified inconsistencies in coding approaches. There are places where the entities are pure abstract class and some concrete. A call to create an entity in the UI returns a business rule object cast as an entity in places and others are simply a concrete instance of the entity class. Also, the BusinessFacade manipulates the OPF’s database objects (bypassing the BusinessRules) where in other places the Façade leaves OPF’s database manipulation to the BusinessRules library.
We recorded some issues with reuse at the BusinessEntities and Rules. We had some issues with serializing business entities over web services.
At the UI level, threading, refreshing and bulky interface are the main issues. A lot of threads access the UI thread and tries to manipulate it. This has resulted in erratic behaviors reported by clients.
The bulky interface is due in parts to the early versions of Infragistics Toolkit and largely design. Forms are bulky, a good example is FrmMainEMR. Opening it in the designer often takes up to a minute. We agreed building the form as a collection of different UI controls would have resulted in lighter, more disconnected and scalable architecture.
Refreshing the UI is often a problem. The problem is now reduced after limiting the result sets and making upgrades to the OPF.
Documentation exists but is not adequate. A lot of public methods exist without proper Xml documentation. Also, some complex logics are not always commented.
For code reuse, EMR started out the SpCore project. This is a library on non-EMR specific functionalities.
We identified unintended misuse regarding OPF; for instance there are places where returning demoralized data via the OPF would have resulted in a lot of performance gain by reducing database calls i.e. loading related records by doing database joins. Junction tables were returned as business entities object where they are not really entities in the system e.g. PatientContact which is a junction between Patients and Contacts.
EMR followed and actually caused some QA standards to be adopted. Scripts are packaged along with the solution.
The immediate issue with QA is the way QA documents are generated with text files or Word documents.
Probably due to the large numbers of developers that have worked on BELLAGIO EMR or management deficits, adherence to Splashers/Artemis coding standard is low. This is partly due to the fact that most of the standard came after BELLAGIO EMR.
Pascal casing for classes and methods is adopted but not 100%. Variable naming is very inconsistent though.
Methods are often very long and lengthy codes are entered into case statements. Logics are repeated in certain places. Some of these issues have since been corrected.
Best practices in string manipulation and formatting are not stringently followed. Object lifetime are not very well managed, a lot of public class variables and static calls exist. Often forms have lots of constructors and codes are not well utilized.
General exception handling blocks litter EMR. These blocks have since been replaced by specific ones and have revealed some developer errors. Before now some of these have been muted by exception handlers.
The middle tier Façade and BusinessRules had try finally blocks everywhere. A side effect of this is lengthy code. Development team have since revised the code using RAII’s techniques for .Net..
The UI is inconsistent. Forms have different appearances and messages boxes carry different captions and behavior. Xp icons are not used everywhere. Forms are often modal and user feedback suggests less usability.
Miscellaneous issues reported include office automation and com interoperability issues, usability of Sql Reporting services as at VS 2003
BELLAGIO EMR (Electronic Medical Record) is a custom solution targeting healthcare providers. Its purpose is to automate common record keeping and retrieval, document management and scheduling. Medical records are patient centric and it uses windows form as a front end.
The core modules of BELLAGIO EMR include patient registration and encounter management module, task/scheduling module and a document module. Others are Form Builder/BELLAGIO EMR survey, EMR Reporting and a couple of other modules are in development.
Architecture
BELLAGIO EMR is built primarily with c#.Net. SqlServer database serves as its backend. Other technologies in use include Sql server Reporting services, Office Automation, Office 2003 Reporting, Xml. Tools and platforms include VS 2003 and Infragistics toolset for the presentation layer.
Authentication and authorization is done with Active directory.
The application has two physical layer: A heavy client and a database server. Logically the partitions include a Windows form UI project, a BusinessFacade project that implements the façade pattern. The BusinessRules class library defines CRUD functions and some entity specific operations, mostly making database calls via an object persistence framework OPF. The ObjectPersistenceFramework (OPF) is an in-house library that abstracts ADO.Net classes and provides method calls like Create(), Update etc. There is a BusinessEntities project that is just a handful of transport objects with public properties for setting and getting member variables. It does some little manipulations on strings, dates and numbers which are sometimes not persisted to the database..
Other class libraries included in the solution is a Utility project containing enumerations, some date and string manipulation functions and some logging functions. The SpCore class library abstract reusable codes and is not directly tied to EMR.
The only visible pattern we identified in EMR is the Façade pattern.
We did identify the solution would could have benefited from an MVC pattern. An Observer pattern using events and delegates would have been ideal for updating the multiple views in EMR, an approach that would make the application more scalable and reduced maintenance headaches. This could also provide a framework for automation and plug-ins
.
We identified inconsistencies in coding approaches. There are places where the entities are pure abstract class and some concrete. A call to create an entity in the UI returns a business rule object cast as an entity in places and others are simply a concrete instance of the entity class. Also, the BusinessFacade manipulates the OPF’s database objects (bypassing the BusinessRules) where in other places the Façade leaves OPF’s database manipulation to the BusinessRules library.
We recorded some issues with reuse at the BusinessEntities and Rules. We had some issues with serializing business entities over web services.
At the UI level, threading, refreshing and bulky interface are the main issues. A lot of threads access the UI thread and tries to manipulate it. This has resulted in erratic behaviors reported by clients.
The bulky interface is due in parts to the early versions of Infragistics Toolkit and largely design. Forms are bulky, a good example is FrmMainEMR. Opening it in the designer often takes up to a minute. We agreed building the form as a collection of different UI controls would have resulted in lighter, more disconnected and scalable architecture.
Refreshing the UI is often a problem. The problem is now reduced after limiting the result sets and making upgrades to the OPF.
Documentation exists but is not adequate. A lot of public methods exist without proper Xml documentation. Also, some complex logics are not always commented.
For code reuse, EMR started out the SpCore project. This is a library on non-EMR specific functionalities.
We identified unintended misuse regarding OPF; for instance there are places where returning demoralized data via the OPF would have resulted in a lot of performance gain by reducing database calls i.e. loading related records by doing database joins. Junction tables were returned as business entities object where they are not really entities in the system e.g. PatientContact which is a junction between Patients and Contacts.
EMR followed and actually caused some QA standards to be adopted. Scripts are packaged along with the solution.
The immediate issue with QA is the way QA documents are generated with text files or Word documents.
Probably due to the large numbers of developers that have worked on BELLAGIO EMR or management deficits, adherence to Splashers/Artemis coding standard is low. This is partly due to the fact that most of the standard came after BELLAGIO EMR.
Pascal casing for classes and methods is adopted but not 100%. Variable naming is very inconsistent though.
Methods are often very long and lengthy codes are entered into case statements. Logics are repeated in certain places. Some of these issues have since been corrected.
Best practices in string manipulation and formatting are not stringently followed. Object lifetime are not very well managed, a lot of public class variables and static calls exist. Often forms have lots of constructors and codes are not well utilized.
General exception handling blocks litter EMR. These blocks have since been replaced by specific ones and have revealed some developer errors. Before now some of these have been muted by exception handlers.
The middle tier Façade and BusinessRules had try finally blocks everywhere. A side effect of this is lengthy code. Development team have since revised the code using RAII’s techniques for .Net..
The UI is inconsistent. Forms have different appearances and messages boxes carry different captions and behavior. Xp icons are not used everywhere. Forms are often modal and user feedback suggests less usability.
Miscellaneous issues reported include office automation and com interoperability issues, usability of Sql Reporting services as at VS 2003
# posted by Unknown @ 2:14 AM 
