public ActionResult Add(FormCollection values)
{
var keywordRepository = new DataRepository<Keyword>();
var customer = new Customer
{
Url = values["Url"],
Slug = SlugGenerator.GetSlug(values["Url"]),
Created = DateTime.Now.Date,
NextReport = DateTime.Now.AddMonths(1).Date,
Interval = int.Parse(values["interval"]),
Due = DateTime.Now.AddMonths(12).Date,
UserId = Guid.NewGuid(),
AnalyticsId = 1,
AnalyticsUsername = "alexander",
AnalyticsPassword = "hejsan"
};
var keywords = values["Keywords"].Split(Environment.NewLine.ToCharArray());
var cleanedKeywords = keywords.Where(x => x.Trim().Length > 0);
foreach (var keyword in cleanedKeywords)
{
var databaseKeyword = keywordRepository.Single(x => x.Name == keyword);
if (databaseKeyword == null)
{
databaseKeyword = new Keyword { Name = keyword };
keywordRepository.Insert(databaseKeyword);
}
customer.Keywords.Add(databaseKeyword);
}
_customerRepository.Insert(customer);
_customerRepository.Commit();
return RedirectToAction("Index");
}
Refactorings
No refactoring yet !
Ants
August 25, 2009, August 25, 2009 01:20, permalink
No rating.
Login to rate!
I subscribe to the idea of thin controllers and so pushing business logic into the Model. So I would do the following with your code above.
I also took out the hardcoded analytics id/username/password and added code to read them from web.config.
class CustomersController
{
:
public ActionResult Add(FormCollection values)
{
var customer = new Customer
{
Url = values["Url"],
Slug = SlugGenerator.GetSlug(values["Url"]),
Interval = int.Parse(values["interval"]),
Analytics = Analytics.CreateFromConfig(),
};
customer.SetKeywords(new DataRepository<Keyword>(),
values["Keywords"].Split(Environment.NewLine.ToCharArray()));
_customerRepository.Insert(customer);
_customerRepository.Commit();
return RedirectToAction("Index");
}
:
}
class Analytics
{
public int Id { get; set; }
public string Username { get; set; }
public string Password { get; set; }
// Assumes the following exist in web.config:
// <appSettings>
// <add key="AnalyticsId" value="1"/>
// <add key="AnalyticsUsername" value="alexander"/>
// <add key="AnalyticsPassword" value="hejsan"/>
// </appSettings>
public static CreateFromConfig()
{
Configuration config = WebConfigurationManager.OpenWebConfiguration(null);
return new Analytics()
{
Id = config.AppSettings.Settings["AnalyticsId"],
Username = config.AppSettings.Settings["AnalyticsUsername"],
Password = config.AppSettings.Settings["AnalyticsPassword"],
};
}
}
class Customer
{
:
public Analytics Analytics { get; set; }
:
private DataRepository<Keyword> _keywordRepository;
:
public Customer()
{
:
this.Created = DateTime.Now.Date;
this.NextReport = this.Created.AddMonths(1).Date;
this.Due = this.Created.AddMonths(12).Date;
this.UserId = Guid.NewGuid();
:
}
:
public void SetKeywords(DataRepository<Keyword> keywordRepository,
IEnumerable<string> keywords)
{
Debug.Assert(_keywordRepository == null || _keywordRepository == keywordRepository,
"Can't support swapping repositories before commit.");
_keywordRepository = keywordRepository;
var cleanedKeywords = keywords.Where(x => x.Trim().Length > 0);
foreach (var keyword in cleanedKeywords)
{
var databaseKeyword = keywordRepository.Single(x => x.Name == keyword);
if (databaseKeyword == null)
{
databaseKeyword = new Keyword { Name = keyword };
keywordRepository.Insert(databaseKeyword);
}
this.Keywords.Add(databaseKeyword);
}
}
:
public void Commit()
{
:
_keywordRepository.Commit();
_keywordRepository = null;
:
}
}
Ants
August 25, 2009, August 25, 2009 23:32, permalink
No rating.
Login to rate!
Even with the above changes, I'm still unhappy that the Customer class knows how Keyword's are stored. My first thought is that it is the DataRepository<Keyword> that should have this responsibility/knowledge. Since C# doesn't have the equivalent of C++ template specialization, options are either to fake it using extension methods, or simply have the Keyword class have the knowledge.
Right now, I'm leaning towards the latter leading to this refactoring:
class CustomersController
{
:
public ActionResult Add(FormCollection values)
{
var customer = new Customer
{
Url = values["Url"],
Slug = SlugGenerator.GetSlug(values["Url"]),
Interval = int.Parse(values["interval"]),
Analytics = Analytics.CreateFromConfig(),
};
customer.Keywords.AddRange(Keyword.LookupKeywords(new DataRepository<Keyword>(),
values["Keywords"].Split(Environment.NewLine.ToCharArray()));
_customerRepository.Insert(customer);
_customerRepository.Commit();
return RedirectToAction("Index");
}
:
}
class Analytics
{
public int Id { get; set; }
public string Username { get; set; }
public string Password { get; set; }
// Assumes the following exist in web.config:
// <appSettings>
// <add key="AnalyticsId" value="1"/>
// <add key="AnalyticsUsername" value="alexander"/>
// <add key="AnalyticsPassword" value="hejsan"/>
// </appSettings>
public static CreateFromConfig()
{
Configuration config = WebConfigurationManager.OpenWebConfiguration(null);
return new Analytics()
{
Id = config.AppSettings.Settings["AnalyticsId"],
Username = config.AppSettings.Settings["AnalyticsUsername"],
Password = config.AppSettings.Settings["AnalyticsPassword"],
};
}
}
class Customer
{
:
public Analytics Analytics { get; set; }
:
public Customer()
{
:
this.Created = DateTime.Now.Date;
this.NextReport = this.Created.AddMonths(1).Date;
this.Due = this.Created.AddMonths(12).Date;
this.UserId = Guid.NewGuid();
:
}
:
}
class Keyword
{
:
public static IEnumerable<Keyword> LookupKeywords(DataRepository<Keyword> keywordRepository,
IEnumerable<string> keywordsText)
{
var keywords = new List<Keywords>();
var cleanedKeywords = keywordsText.Where(x => x.Trim().Length > 0);
foreach (var keyword in cleanedKeywords)
{
var databaseKeyword = keywordRepository.Single(x => x.Name == keyword);
if (databaseKeyword == null)
{
databaseKeyword = new Keyword { Name = keyword };
keywordRepository.Insert(databaseKeyword);
}
keywords.Add(databaseKeyword);
}
keywordRepository.Commit();
return keywords;
}
:
}
James Jensen
October 5, 2009, October 05, 2009 20:32, permalink
No rating.
Login to rate!
Adding on to Ants's response: the query for cleanedKeywords trims the string but you never use trimmed strings thereafter. The following allows you to use the trimmed keywords in interacting with your repository. Also, I don't know how you prefer to handle duplicates, but it seems like you want them to be unique.
Also, if your keyword repository has costly round-trips (like a database or web service), you may want to use LinqKit's .WhereIn extension method so that you can pull all the necessary keywords from the repository in a single round-trip.
using LinqKit;
public static IEnumerable<Keyword> LookupKeywords(DataRepository<Keyword> keywordRepository,
IEnumerable<string> keywordsText)
{
var keywords = new List<Keyword>();
var cleanedKeywords = keywordsText.Select(x => x.Trim())
.Where(x => Length > 0)
.Distinct();
var repositoryKeywords = keywordRepository.WhereIn(x => x.Name, cleanedKeywords)
.ToDictionary(x => x.Name);
foreach (var keyword in cleanedKeywords)
{
Keyword repositoryKeyword;
if (!repositoryKeywords.ContainsKey(keyword))
{
repositoryKeyword = new Keyword { Name = keyword };
keywordRepository.Insert(repositoryKeyword);
}
else
{
repositoryKeyword = repositoryKeywords[keyword];
}
keywords.Add(databaseKeyword);
}
keywordRepository.Commit();
return keywords;
}
flytaeades
October 31, 2009, October 31, 2009 17:24, permalink
No rating.
Login to rate!
smaller near organizations overwhelming circulation energy colleagues assessment
ledoFlienimew
January 16, 2010, January 16, 2010 08:02, permalink
No rating.
Login to rate!
test
http://gooogle.com
I have this method wich is invoked on a post request to a action method. What it does is adding a new customer to the database, and adds several keywords to the customer.
On every request, i'm checking if a keyword already is in the database, if so, just add a reference to the customer, otherwise insert it.
There is a many-to-many relationship between company and keywords. The keywords is newline-separated in a textarea.
Refactorings? Thanks