Skip to content

Commit

Permalink
readding script version resolver to the complex version resolver list…
Browse files Browse the repository at this point in the history 
…. Just passing config prop holder to the constructor to let the class be smarter about setting itself up
  • Loading branch information
@ferventcoder
ferventcoder committed Oct 23, 2011
1 parent 04b07dd commit 663f6c2
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 27 deletions.
43 changes: 19 additions & 24 deletions product/roundhouse/infrastructure.app/builders/VersionResolverBuilder.cs
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
namespace roundhouse.infrastructure.app.builders
{
using System.Collections.Generic;
using filesystem;
using resolvers;

public class VersionResolverBuilder
{
public static VersionResolver build(FileSystemAccess file_system, ConfigurationPropertyHolder configuration_property_holder)
{
VersionResolver xml_version_finder = new XmlFileVersionResolver(file_system,
configuration_property_holder.VersionXPath,
configuration_property_holder.VersionFile);
VersionResolver dll_version_finder = new DllFileVersionResolver(file_system,
configuration_property_holder.VersionFile);

VersionResolver script_number_version_finder = new ScriptfileVersionResolver(file_system,
configuration_property_holder.VersionFile,
file_system.combine_paths(configuration_property_holder.SqlFilesDirectory, configuration_property_holder.UpFolderName));
IEnumerable<VersionResolver> resolvers = new List<VersionResolver> {xml_version_finder, dll_version_finder};

return new ComplexVersionResolver(resolvers);
}
}
namespace roundhouse.infrastructure.app.builders
{
using System.Collections.Generic;
using filesystem;
using resolvers;

public class VersionResolverBuilder
{
public static VersionResolver build(FileSystemAccess file_system, ConfigurationPropertyHolder configuration_property_holder)
{
VersionResolver xml_version_finder = new XmlFileVersionResolver(file_system, configuration_property_holder.VersionXPath, configuration_property_holder.VersionFile);
VersionResolver dll_version_finder = new DllFileVersionResolver(file_system, configuration_property_holder.VersionFile);
VersionResolver script_number_version_finder = new ScriptfileVersionResolver(file_system, configuration_property_holder);

IEnumerable<VersionResolver> resolvers = new List<VersionResolver> { xml_version_finder, dll_version_finder, script_number_version_finder };

return new ComplexVersionResolver(resolvers);
}
}
}
8 changes: 5 additions & 3 deletions product/roundhouse/resolvers/ScriptfileVersionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@

namespace roundhouse.resolvers
{
using infrastructure.app;

public class ScriptfileVersionResolver : VersionResolver
{
private string version_file;
private FileSystemAccess file_system;
private string up_folder;

public ScriptfileVersionResolver (FileSystemAccess file_system, string version_file, string upFolder)
public ScriptfileVersionResolver(FileSystemAccess file_system, ConfigurationPropertyHolder configuration_property_holder)
{
this.version_file = version_file;
this.file_system = file_system;
this.up_folder = upFolder;
this.version_file = configuration_property_holder.VersionFile;
this.up_folder = file_system.combine_paths(configuration_property_holder.SqlFilesDirectory, configuration_property_holder.UpFolderName);

This comment has been minimized.

Sign in to view

Copy link
@developingchris

developingchris Oct 23, 2011

Contributor

Doesn't this overcomplicate the responsibility of this version resolver? Since now you must depend on a full configuration before you can resolve any version.
I realize its 6 or a 1/2 dozen given that this object is used 1 time and that usage always has a configuration object in scope. But given our previous discussion about simplicity and design, this seems counter to that end.
Thoughts?

This comment has been minimized.

Sign in to view

Copy link
@ferventcoder

ferventcoder Oct 23, 2011

Author Member

I would almost agree. Yes, it is a little more responsibility on the resolver, but in configuring itself given an object. I don't necessarily see that as too much responsibility since like you said, it is kind of splitting hairs.

The idea here leans towards OCP in that if later we want another configuration value passed we would need to expand the constructor, changing the code in in two places. Since two of those items can be pulled by still passing the configuration, it makes sense to reduce that down to just passing the object itself instead of primitives values of the object. Then we insulate changes a bit more, which is great for maintenance.

Does that make sense?

This comment has been minimized.

Sign in to view

Copy link
@ferventcoder

ferventcoder Oct 23, 2011

Author Member

And yes, there are more places in the code that would lean well to exploring the concept of constructor injection reduction. I reserve the right to contradict myself. ;)
}

public bool meets_criteria()
Expand Down
1 change: 1 addition & 0 deletions product/roundhouse/roundhouse.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
<Compile Include="parameters\AdoNetParameter.cs" />
<Compile Include="parameters\IParameter.cs" />
<Compile Include="Migrate.cs" />
<Compile Include="resolvers\ScriptfileVersionResolver.cs" />
<Compile Include="RoundhousEFluentNHibernateDiffingType.cs" />
<Compile Include="runners\RoundhouseNHibernateCompareRunner.cs" />
<Compile Include="runners\RoundhouseRedGateCompareRunner.cs" />
Expand Down

0 comments on commit 663f6c2

Please sign in to comment.