18F / open-data-maker

make it easy to turn a lot of potentially large csv files into easily accessible open data
Other
199 stars 135 forks source link

Refactor: Extracts some objects from DataMagic::Config #300

Open baccigalupi opened 8 years ago

baccigalupi commented 8 years ago

When I put through my last pull request with refactoring on the index process, I realized there was likely some fuzziness around the document building process, in the DocumentBuilder, and other parts of the newly extracted indexing code.

When I looked around in that module, I realized that there was a lot of stuff inside Config that was being called from here. I also realized that the Config object was not a value object that helped the rest of the application along, but an incredibly active universe with file loading, logic to switch between S3 and local file access, parsing and normalizing the configuration data. This thing is more like a configuration builder, and I felt like I needed to understand it and take it apart before I could fix the abstraction confusion in the DocumentBuilder.

This set of commits does two main things:

  1. Extracts a value object from the configuration for storing the seemingly important data in the Config object. This could be used in the future as real config value object and passed to processes that need it.
  2. Extracts the loading of configuration files. The process of loading files is pretty complicated, and it is hard to tell what the product goals are. I did see two different file adapters for S3 and local, along with a container that could read the files from wherever and then translate the content to YAML.

I know the perception is that this is not an area needing a lot of refactor help, or at least not as much as the area I was focused on. If you checkout code climate: https://codeclimate.com/github/18F/open-data-maker/trends. You can see that the Config module is the current file with the combination of greatest churn and lowest code quality. That is a pretty good indication that it is costing active development money through its quality.