For my javascript project I was looking for an automated tool that would merge all my files into one, which then could be optimized by YUI compressor. I found combiner project developed by nzakas which used require-in-comment approach which was close to what I needed – my JS library uses require function for importing dependencies.

After forkeing it and started code analysis as well as other forks to see what changes would I need to make to adjust it to my needs and it looked like my improvement plan had the same points as most of the forks although I was planning to keep it as simple as possible, yet expandable.

So the plan was:

  • replace jargs with args4j
  • add generics to all collections
  • improve sorting and duplicates handling on the output collection by using TreeSet instead of List and proper Comparator
  • improve cyclic dependency management
  • improve files reading into SourceFile – remove todo List that was used for data exchange between processFile/processFiles methods
  • move file handling functionality into a subclass and thus enable fast exchange of the dependency algorithm – use it for css @import statement

First two points were no-brainers, so they went smoothly.

Sorting/duplicates removal

My biggest problem was the sorting algorithm (3rd point) – in the original version there was List with „manual” checks using List.contains function and after list has been filled with data it was sorted by using Collections.sort and appropriate Comparator.

This all worked fine but I was sure that the List could be replaced with TreeSet where both order and duplicates would be handled at once but original Comparator was a problem. It was good for sorting but not for insertion – when it found two non-related files with equal dependency size it returned 0 which in TreeSet terms meant that file would not be inserted  AT ALL. I checked TreeSet.add method and I found out that it uses binary search algorithm – so there are 2 cases to handle:

  • either there is no more elements, which meant element was inserted
  • Comparator returned 0, which meant that element was replaced.

And this was it – I just had to make sure that elements with the same number of dependencies are handled consistently until there is no more elements or the same element is found. This meant comparing 2 non-related element with the same number of depedencies by name.

Get rid of todo property

Second thing was processSourceFiles method that used extra list in order to exchange dependencies found by processSourceFile method. This looked weird from the beginning, so I was wondering if adding a return value to processSourceFile would help. Afterwards I noticed that processSourceFiles can be called recursively and thus letting go todo list.

Simplify cyclic dependency management

Than there was cyclic dependency handling – there used to be an extra loop that would check all files one-by-one if any of them is not already dependent on currently processed one. So I figured that it would be much easier to do the check inside addDependency which meant getting rid of all overloaded methods and leaving just one that accepts SourceFile object. Finally addDependency method checks if current object is already a dependency of added object (passed as parameter) and if it is, it returns false.

Subclass for reading files

So than removing file reading algorithm and adding Css handler was really easy because dependency reading was already fleshed out, so baiscally I just had to move it outside FileComparator class.

2 Comments

  1. Krzysiek says:

    For asset management there is a really great tool called Assetic. It is going to be part of brand new framework Symfony 2. ATM it is in development stage however so far looks very well, taking into account the fact that the project was started one month ago.

    Repo: https://github.com/kriswallsmith/assetic
    More about: http://www.slideshare.net/kriswallsmith/introducing-assetic-asset-management-for-php-53

  2. Krzysiek says:

    Fail. I thought you’ve been writing about PHP code. Turns out it was Java:P

Leave a Reply