Technical Blog

2 Posts tagged with the code tag

Code review is a good way to catch mistakes, review designs, and help developers learn from each other.  Over-the-shoulder reviews are an easy habit to enforce for small teams, where developers are only a chair slide away.  With more projects spanning into multiple teams that has developers working with partners an ocean apart, code reviews become more of an inconvenience.  There are many code review tools out there to help with this problem.  For one of our projects, our customer introduced us to the Code Collaborator tool, which makes code reviews over the wire a lot easier.

 

HOW IT WAS DONE BEFORE…

 

When developers are not working in the same office, it makes sharing code a little harder.  A common practice is to commit the code and have the reviewer rely on the revision logs to find all the changes.  This can quickly become a headache if the changes span over multiple commits.  It also means that you are committing potentially buggy or smelly code to the build, which should make any good developer squeal.

 

Another way is to send code directly through emails.  The downside, being, you will have to download each file to your machine, and manually diff each one of them to find what the changes are.  This is also a great way to fill up your inbox, with code conversations and attachments going back and forth.

 

THE TOOL!

 

Smart Bear's Code Collaborator is a peer review tool that automates many parts of the code review process and also cuts down some of the little annoyances.  You can easily connect it to your favourite version control system, and have it collect all the code changes you have made, without the need to commit actual code.  This can be done by either using the tool’s GUI client, or through the various plug-ins available for existing source control clients.  Perforce 4 was the VCS of our project.  We simply installed the plug-in available for P4V client, and we were able to create reviews with just a few clicks.

 

perforce-p4v-changelistmenu.zoom50.png

 

As a developer uploads the code to be reviewed, he can specify who should participate in the review.  Once done, he can simply create the review and continue making changes to the code and upload the changes to the same review later, or begin the code review process.

 

CC3.png

 

Once submitted for review, the participants are notified through email.  They can then login to the server and begin looking through the code changes.  They can also see any outstanding reviews that they need to perform.

 

CC4.png

 

A reviewer can comment specific lines of code, or even mark defects found.  One can even track a defect to an external issue tracker.  Conversations between the reviewer and author are also tracked per highlighted line.

 

CC5.png

 

Admittedly, the notification emails can easily spam up your inbox if you are reviewing for multiple people.  We found email filters essential for taming the tool.  Otherwise, all the conversations, code changes and issues found can be managed inside the tool.  Being able to review code without any commits increases collabortaion without any risk of having the CI build break.  This tool also has a search function that let’s you search for particular comments within your reviews, making it easier to back trace reasons for changes.  There are still many features that we haven’t fully explored yet.

 

Overall, we found this tool to be very helpful for maintaining our code quality, as well as for transfering knowledge and keeping team members aware of all the project changes.  There are also many alternatives to Code Collaborator, some of which are free.  Atlassian’s Crucible and Google’s code review app both have similar functionality.  Code review tools like these are worth considering for larger teams.

0 Comments Permalink

Many classes were refactored in Elastic Path 6.1 to improve testability of existing code and to accommodate new features (most notably the ability for a single Storefront WAR file to host multiple Stores). Here are the changes with the highest impact for developers (in no particular order).

1. Refactoring of ElasticPathImpl.java and Creation of the Settings Framework

ElasticPathImpl covers several distinct areas of responsibility: BeanFactory, Configuration, Filtered Navigation, asset lookup... In 6.1, new interfaces were created to move those areas of functionality out of ElasticPathImpl with the intention of rendering the object obsolete in future versions.

  • Most configuration settings are now accessed via the SettingsService, which will be covered in a separate article. Commerce-config.xml files are no longer used to populate a map of settings in ElasticPathImpl, with the exception of the one in the CM web application, which is only used to keep track of the EP version and build number
  • Asset locations are now retrieved from the AssetRepository interface (see heading elsewhere in this article)
  • Search Configuration is now retrieved entirely from the Settings framework rather than from the ElasticPath object
  • Filtered Navigation information is still retrieved from ElasticPathImpl, but only because it implements the new FilteredNavigationConfiguration interface. The actual retrieval implementation now lives in a separate class and references the new Settings framework
  • Bean instances can still be retrieved from ElasticPath because ElasticPathImpl implements the BeanFactory interface, but from this point onward the ElasticPath object is not required to be injected into classes that only need a BeanFactory. Instead, you should inject an implementation of BeanFactory

If any customizations were performed on some Elastic Path 6.0 code base that must be transferred to an Elastic Path 6.1 code base, most careful attention should be paid to any changes in the ElasticPath singleton, especially if any new settings were introduced that are designed to be read in from a commerce-config.xml configuration file. Since most settings were moved to the Settings framework, the code to parse the configuration file has mostly been removed.

New customizations should pay special attention to the Settings framework and use it where appropriate. Also, the BeanFactory should be injected into new classes where required instead of the ElasticPath object, as its use as a bean factory is being phased out.

New persistent classes should have a DAO that is separate from any new services, to allow for more flexible persistence implementations if functionality changes.

2. Testing Framework Enhancements

In testing, the big news is deprecation of ElasticPathTestCase.java and the introduction of JUnit4 and JMock2 testing frameworks.

Prior to 6.1, many test classes extended ElasticPathTestCase.java, which is a JUnit 3.8 extended test class that mocks out much of the application's domain using mock objects created with the JMock 1.x framework. This class is being phased out, and use of it is no longer recommended because it:

  • is large and unwieldy
  • encourages poor code factoring and integration tests using mock objects
  • is complex; modification may affect many test cases in an unforeseen manner

 

Instead, unit test classes should focus on unit-testing rather than integration-testing methods. Use of the JUnit4 and JMock2 framework features should help in that regard. There are many examples of their use in the new code; but for a start you may want to look at GiftCertificateThemeImplTest (for a relatively simple test class) and StoreResourceManagerImplTest (which takes advantage of some advanced features in the new JMock framework).

3. New Store UI Themes and Asset Retrieval

Store-specific Velocity template resolution has been introduced. Most static and storefront theme content (storefront velocity templates, email velocity templates, javascript files, css files, Power Reviews, etc) was moved outside the separate application WAR files into the pre-existing Assets folder on the file system.

 

In 6.0 only some assets (store images and digital goods) lived on the file system under the assets folder, while others (email templates, storefront UI files, and PowerReviews files) lived inside the WAR. All of these files were moved to appropriate locations under the file system's assets folder and the paths to such folders were made available through an AssetRepository object rather than through methods on the ElasticPath object. This allows the assets to be modified at runtime.

 

The Storefront application's Themes feature (see Themes documentation) required the introduction of a StoreResourceManager to handle Velocity's lookups and caching for Storefront velocity templates. It does not use the AssetRepository, but instead uses very similar code combined with hardcoding the velocity template directory within the assets folders (see StoreVelocityConfigHelper.java). A separate Velocity Engine is created for each and every Storefront operating within a WAR.

The Storefront's asset requests that are not for Velocity templates (e.g. css, javascript, images, Power Reviews) go through the AssetResourceController, which uses the AssetRepository indirectly through an AssetRetrievalStrategy. In order for controllers to know which store they are working with, a ThreadLocal reference to Store has been introduced, set by the StoreSelectionFilter. Any storefront controller requiring parameters specific to the store that is accessing the controller can inject the StoreConfig object, or they can retrieve it from the RequestHelper, which also has it injected. The current store is no longer retrieved from ElasticPath.java.

4. New DAO classes

New DAO classes were introduced in some areas to separate service logic from persistence operations. In addition to some new features using separate Service and DAO classes, existing Services which have had DAO functionality factored out include:


  • ProductService / ProductDao
  • ProductTypeService / ProductTypeDao

 

DAO classes should generally not be accessed directly from outside of the Service layer. Such separation of business logic and persistence logic allows for far more flexibility and decoupling in the application. Future development should endeavor to follow this pattern (See the GiftCertificateThemeService / GiftCertificateThemeStoreAssignmentDao for an example of how such decoupling can be of benefit).

5. Index UID / GUID fields

We are phasing out UID fields in the indexes, in favor of GUID fields, for greater decoupling. Secondary object UID fields have, for the most part, been replaced with GUID fields in object indexes so that searches can be performed based on GUIDs. For instance, the CategoryIndex contains the CategoryUid so that the UID can be retrieved from the index when a search match is found, but the index is built using the GUIDs from associated objects (e.g. you can search the CategoryIndex to return categories that are associated with a particular CatalogCode, but not with a particular CatalogUid). The corresponding SearchCriteria objects have been modified to reflect the index schema changes.

Index fields that have been modified as a result of this change are as follows:


  • CategoryIndex:

 

  • parentCategoryUids replaced with parentCategoryCodes
  • catalogUid replaced with catalogCode

 

  • PromotionIndex

 

  • promotionRuleSetUid continues to live in the index, but the promotionRuleSetName is now also in the index. This is for legacy reasons (the PromotionSearchCriteria object still contains both fields as well), and the UID field is expected to be removed in a future version of Elastic Path.
  • catalogUid continues to live in the index, but catalogCode is now also in the index. This is for legacy reasons (the PromotionSearchCriteria object still contains both fields as well), and the UID field is expected to be removed in a future version of Elastic Path.

 

  • ProductIndex

 

  • CategoryUid replace with CategoryCode
  • parentCategoryUids replaced with parentCategoryCodes
  • catalogUid replaced with catalogCode
  • Addition of a new storeCode field

 

In addition, several fields were added to the OrderReturn index schema.

6. Improved Index Build Status Tracking

The status of indexes, and the method of causing them to rebuild, has changed drastically:


  • The index build status (including the last time the index was built) is stored in the new TINDEXBUILDSTATUS database table, and can be accessed using the IndexBuildStatusService.
  • On the Search Server, the AbstractIndexServiceImpl class manipulates the status of index builds directly through the IndexBuildStatusDao.
  • The Commerce Manager Client has a new View that enables index rebuilds to be triggered manually.

 

In addition, if the server is brought down (either manually or catastrophically) while in the process of building an index, it will now remember its status upon restart and will start a new rebuild rather than assuming that the index is complete.

7. New Store Editor GUI

The Store Editor has been enhanced to better handle store themes. In addition, it has abstracted most of the business logic from where it may previously have been embedded in the UI code.

0 Comments 0 References Permalink