I have a switch statement that works just fine, however, it has a lot of duplicate logic within it. I have an enum that my controllers use to say which is making the publish request, so ProductInfo
controller passes in ContentFields.ProductInformation
into PublishTable
public static void PublishTable(string user, ContentFields tableToPublish)
{
var jsonData = DataAccess.GetEnvironmentJson((int)Environments.Production);
var model = JsonConvert.DeserializeObject<Models.FullDataSetModel>(jsonData);
using (var db = new LoginPageContentEntities())
{
var deployEntry = db.Deployment.Find((int)Environments.Production);
deployEntry.DateUpdated = DateTime.Now;
switch (tableToPublish)
{
case ContentFields.ProductInformation:
deployEntry.ProductInformation = false;
model.ProductInformation.Clear();
model.ProductInformation = db.ProductInformation.ToList();
break;
case ContentFields.DidYouKnow:
deployEntry.DidYouKnow = false;
model.DidYouKnow.Clear();
model.DidYouKnow = db.DidYouKnow.ToList();
break;
case ContentFields.MaintenanceMessage:
deployEntry.MaintenanceMessage = false;
model.MaintenanceMessage.Clear();
model.MaintenanceMessage = db.MaintenanceMessage.ToList();
break;
case ContentFields.TrainingEvent:
deployEntry.TrainingEvent = false;
model.TrainingEvents.Clear();
model.TrainingEvents = db.TrainingEvents.ToList();
break;
case ContentFields.VideoContent:
deployEntry.VideoContent = false;
model.HtmlSnippets.Clear();
model.HtmlSnippets = db.HtmlSnippets.ToList();
break;
case ContentFields.ProductMarketingUrl:
deployEntry.ProductMarketingUrl = false;
model.ProductMarketingUrl.Clear();
model.ProductMarketingUrl = db.ProductMarketingUrl.ToList();
break;
}
var json = JsonConvert.SerializeObject(model);
deployEntry.JsonCache = json;
db.SaveChanges();
}
}
Model:
public class FullDataSetModel
{
public List<DidYouKnow> DidYouKnow { get; set; }
public List<HtmlSnippets> HtmlSnippets { get; set; }
public List<MaintenanceMessage> MaintenanceMessage { get; set; }
public List<ProductInformation> ProductInformation { get; set; }
public List<TrainingEvents> TrainingEvents { get; set; }
public List<ProductMarketingUrl> ProductMarketingUrl { get; set; }
}
My question is how can i remove the need for the switch here while being able to specify which part of the model to update. Or, whether that is even needed and a switch is ok here even though there is a lot of duplicate logic. More or less this comes down to design, I feel this is pretty human readable to understand what is happening but it may not be the most efficient way to handle this code.
As an aside if this was to be a database context rather than a model such as:
public static void RevertChanges(Environments targetToRestoreFrom, ContentFields tableToRevert)
{
var jsonData = DataAccess.GetEnvironmentJson((int)targetToRestoreFrom);
var model = JsonConvert.DeserializeObject<Models.FullDataSetModel>(jsonData);
using (var db = new LoginPageContentEntities())
{
switch (tableToRevert)
{
case ContentFields.ProductInformation:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [ProductInformation]");
db.ProductInformation.AddRange(model.ProductInformation);
break;
case ContentFields.DidYouKnow:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [DidYouKnow]");
db.DidYouKnow.AddRange(model.DidYouKnow);
break;
case ContentFields.MaintenanceMessage:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [MaintenanceMessage]");
db.MaintenanceMessage.AddRange(model.MaintenanceMessage);
break;
case ContentFields.TrainingEvent:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [TrainingEvents]");
db.TrainingEvents.AddRange(model.TrainingEvents);
break;
case ContentFields.VideoContent:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [HtmlSnippets]");
db.HtmlSnippets.AddRange(model.HtmlSnippets);
break;
case ContentFields.ProductMarketingUrl:
db.Database.ExecuteSqlCommand("TRUNCATE TABLE [ProductMarketingUrl]");
db.ProductMarketingUrl.AddRange(model.ProductMarketingUrl);
break;
}
db.SaveChanges();
}
}
How could one remove the switch here and still specify which entity the context should be referencing?
-
\$\begingroup\$ Welcome to Code Review. Since this question was cross-posted from Stack Overflow, I recommend that you add the link there and here. I went ahead and fetched the link: stackoverflow.com/questions/32012909/… \$\endgroup\$Ismael Miguel– Ismael Miguel2015年08月14日 15:11:34 +00:00Commented Aug 14, 2015 at 15:11
-
\$\begingroup\$ Ah great, I assumed my question would be closed there as off topic so I didn't bother getting it (as the link would be dead). \$\endgroup\$Zach M.– Zach M.2015年08月14日 15:12:16 +00:00Commented Aug 14, 2015 at 15:12
-
\$\begingroup\$ It would be closed, but not deleted. Everybody can still see it. Also, the link there, would avoid people from spamming you to move the question to Code Review, even if it already was (like in your case) \$\endgroup\$Ismael Miguel– Ismael Miguel2015年08月14日 15:13:45 +00:00Commented Aug 14, 2015 at 15:13
-
\$\begingroup\$ Can you use reflection? It may be a bit slower, but more readable and maintainable. Other option would be to have array of object lists and do some casting, but have universal access to internal array holding data for the properties. I would use reflection. \$\endgroup\$user52292– user522922015年08月14日 15:20:17 +00:00Commented Aug 14, 2015 at 15:20
-
\$\begingroup\$ I will look into that, I am not very familiar with reflection. \$\endgroup\$Zach M.– Zach M.2015年08月14日 15:21:30 +00:00Commented Aug 14, 2015 at 15:21
1 Answer 1
There is one big thing that makes your code hard to refactor to something prettier namely the ContentFields
enum. It contains two values that are incompatible with the model:
ContentFields.TrainingEvent
vs.model.TrainingEvents
ContentFields.VideoContent
vs.model.HtmlSnippets
This makes it practically impossible to build the SqlCommand
dynamicly or to get the properties by reflection.
If you cannot rename the values you might consider adding custom attributes and get the right name from there (refer to Getting attributes of Enum's value and Creating Custom Attributes:
ContentField
{
[ModelPropertyName("TrainingEvents")]
TrainingEvent
}
The last thing is the name of the ContentFields
enum. If it's not marked with the [Flags]
attribute (you're not using it like that) then it shouldn't be in plural.