patterncsharpMinor
Conditional string building
Viewed 0 times
stringbuildingconditional
Problem
Essentially I'm taking a number of checkboxes off of my view, adding them to a
I'm guessing there must be a better way of doing this.
Here is the complete
```
[HttpGet]
[AllowCrossSiteJsonAttribute]
public JsonpResult CommunicateCard(CommunicateCardModel ccm)
{
if (ModelState.IsValid)
{
StringBuilder builder = new StringBuilder();
// ccm.name is a required field, so we don't have to check if it's empty.
builder.Append("Name: " + ccm.name + "\n");
// If these properties are empty, we don't want to include them in the built string.
builder.Append((!string.IsNullOrEmpty(ccm.address)) ? "Address: " + ccm.address + "\n" : null);
builder.Append((!string.IsNullOrEmpty(ccm.city)) ? "City: " + ccm.cit
Dictionary, and writing out the Keys of any checkbox that is checked.I'm guessing there must be a better way of doing this.
// We need to build a list of what the user would like more information on.
// First step is to add all of the Checkboxes to a `Dictionary`
Dictionary moreInfo = new Dictionary();
moreInfo.Add("Children's Programs", ccm.moreInfoChildren);
moreInfo.Add("Youth Programs",ccm.moreInfoYouth);
moreInfo.Add("Small Groups", ccm.moreInfoSmallGroups);
moreInfo.Add("Getting involved in Ministry",ccm.moreInfoMinistry);
moreInfo.Add("Receiving offering envelopes",ccm.moreInfoOfferingEnvelopes);
moreInfo.Add("Church Membership",ccm.moreInfoMembership);
moreInfo.Add("Baptism",ccm.moreInfoBaptism);
moreInfo.Add("Other (see notes)", ccm.moreInfoOther);
// Second step is to loop through the moreInfo `Dictionary` and build a string of results.
StringBuilder _moreInfo = new StringBuilder();
foreach (var info in moreInfo) {
_moreInfo.Append(info.Value ? info.Key + ", " : "");
}
// Third step is to determin if the user wants any information. If they do, we add that information to the email.
if(!string.IsNullOrEmpty(_moreInfo.ToString()))
{
builder.Append("I would like more information about ");
builder.Append(_moreInfo.ToString());
}Here is the complete
Action:```
[HttpGet]
[AllowCrossSiteJsonAttribute]
public JsonpResult CommunicateCard(CommunicateCardModel ccm)
{
if (ModelState.IsValid)
{
StringBuilder builder = new StringBuilder();
// ccm.name is a required field, so we don't have to check if it's empty.
builder.Append("Name: " + ccm.name + "\n");
// If these properties are empty, we don't want to include them in the built string.
builder.Append((!string.IsNullOrEmpty(ccm.address)) ? "Address: " + ccm.address + "\n" : null);
builder.Append((!string.IsNullOrEmpty(ccm.city)) ? "City: " + ccm.cit
Solution
Your method has a lot of repetition to build your string often resulting in unnecessary calls to
Append(). You should have used an if block for each of those writes instead of appending null. This also hides all the calls to IsNullOrEmpty() which is a detail you don't really want repeated so much. I'd break this into a couple of methods, one for each of the valid/invalid cases and one to help with writing the strings. Written this way, you have the option to use the same trick to quickly build the comma-separated list to write your content easier to do. I think you'll agree that it's a lot more easier to read.[HttpGet]
[AllowCrossSiteJsonAttribute]
public JsonpResult CommunicateCard(CommunicateCardModel ccm)
{
return ModelState.IsValid
? CommunicateValidCard(ccm)
: CommunicateInvalidCard(ccm);
}
StringBuilder AppendNonWhiteField(StringBuilder builder, string format, string field)
{
if (!String.IsNullOrWhiteSpace(field))
{
builder.AppendFormat(format, field);
}
return builder;
}
JsonpResult CommunicateValidCard(CommunicateCardModel ccm)
{
StringBuilder builder = new StringBuilder();
// ccm.name is a required field, so we don't have to check if it's empty.
builder.Append("Name: " + ccm.name + "\n");
// If these properties are empty, we don't want to include them in the built string.
AppendNonWhiteField(builder, "Address: {0}\n", ccm.address);
AppendNonWhiteField(builder, "City: {0}\n", ccm.city);
AppendNonWhiteField(builder, "Postal Code: {0}\n", ccm.postalCode);
AppendNonWhiteField(builder, "Phone Number: {0}\n", ccm.phone);
AppendNonWhiteField(builder, "Email Address: {0}\n", ccm.emailFrom);
builder.AppendLine();
AppendNonWhiteField(builder, "Please {0}\n", ccm.information);
AppendNonWhiteField(builder, "I {0}\n", ccm.christianity);
AppendNonWhiteField(builder, "I am {0}\n", ccm.maritalStatus);
builder.AppendLine();
// pull together all of the checkboxes within the "Ministries" fieldset and generate a single string.
var pairs = new[]
{
// Tuple.Create(string, bool)
Tuple.Create("Children's Programs", ccm.moreInfoChildren),
Tuple.Create("Youth Programs", ccm.moreInfoYouth),
Tuple.Create("Small Groups", ccm.moreInfoSmallGroups),
Tuple.Create("Getting involved in Ministry", ccm.moreInfoMinistry),
Tuple.Create("Receiving offering envelopes", ccm.moreInfoOfferingEnvelopes),
Tuple.Create("Church Membership", ccm.moreInfoMembership),
Tuple.Create("Baptism", ccm.moreInfoBaptism),
Tuple.Create("Other (see notes)", ccm.moreInfoOther),
};
var moreInfo = String.Join(", ", pairs.Where(pair => pair.Item2).Select(pair => pair.Item1));
AppendNonWhiteField(builder, "I would like more information about {0}\n", moreInfo);
AppendNonWhiteField(builder, "I am visiting today because {0}\n", ccm.guestReason);
AppendNonWhiteField(builder, "I am attending for my {0}\n", ccm.attendance);
builder.AppendLine();
AppendNonWhiteField(builder, "{0}", ccm.comments);
// initiate the email service and fire off an email
EmailService email = new EmailService();
email.to = ccm.emailTo;
email.from = ccm.emailTo;
email.replyTo = ccm.emailFrom;
email.subject = ccm.messageSubject;
email.messageBody = builder.ToString();
bool sent = email.send();
object result = null;
// Send a system error message via JSON back to the app if the email fails.
if (!sent)
{
result = new ErrorsModel { ErrorMessage = Server.GetLastError().Message };
}
// whether or not the email sent, we need to tell the app the result.
return this.Jsonp(result, sent);
}
JsonpResult CommunicateInvalidCard(CommunicateCardModel ccm)
{
// The ModelState is invalid, Loop through the ModelState and send a JSON string listing all the errors.
var errors = ModelState.Values
.SelectMany(modelState =>
modelState.Errors
.Select(error => new ErrorsModel { ErrorMessage = error.ErrorMessage }))
.ToList();
return this.Jsonp(errors, false);
}Code Snippets
[HttpGet]
[AllowCrossSiteJsonAttribute]
public JsonpResult CommunicateCard(CommunicateCardModel ccm)
{
return ModelState.IsValid
? CommunicateValidCard(ccm)
: CommunicateInvalidCard(ccm);
}
StringBuilder AppendNonWhiteField(StringBuilder builder, string format, string field)
{
if (!String.IsNullOrWhiteSpace(field))
{
builder.AppendFormat(format, field);
}
return builder;
}
JsonpResult CommunicateValidCard(CommunicateCardModel ccm)
{
StringBuilder builder = new StringBuilder();
// ccm.name is a required field, so we don't have to check if it's empty.
builder.Append("Name: " + ccm.name + "\n");
// If these properties are empty, we don't want to include them in the built string.
AppendNonWhiteField(builder, "Address: {0}\n", ccm.address);
AppendNonWhiteField(builder, "City: {0}\n", ccm.city);
AppendNonWhiteField(builder, "Postal Code: {0}\n", ccm.postalCode);
AppendNonWhiteField(builder, "Phone Number: {0}\n", ccm.phone);
AppendNonWhiteField(builder, "Email Address: {0}\n", ccm.emailFrom);
builder.AppendLine();
AppendNonWhiteField(builder, "Please {0}\n", ccm.information);
AppendNonWhiteField(builder, "I {0}\n", ccm.christianity);
AppendNonWhiteField(builder, "I am {0}\n", ccm.maritalStatus);
builder.AppendLine();
// pull together all of the checkboxes within the "Ministries" fieldset and generate a single string.
var pairs = new[]
{
// Tuple.Create(string, bool)
Tuple.Create("Children's Programs", ccm.moreInfoChildren),
Tuple.Create("Youth Programs", ccm.moreInfoYouth),
Tuple.Create("Small Groups", ccm.moreInfoSmallGroups),
Tuple.Create("Getting involved in Ministry", ccm.moreInfoMinistry),
Tuple.Create("Receiving offering envelopes", ccm.moreInfoOfferingEnvelopes),
Tuple.Create("Church Membership", ccm.moreInfoMembership),
Tuple.Create("Baptism", ccm.moreInfoBaptism),
Tuple.Create("Other (see notes)", ccm.moreInfoOther),
};
var moreInfo = String.Join(", ", pairs.Where(pair => pair.Item2).Select(pair => pair.Item1));
AppendNonWhiteField(builder, "I would like more information about {0}\n", moreInfo);
AppendNonWhiteField(builder, "I am visiting today because {0}\n", ccm.guestReason);
AppendNonWhiteField(builder, "I am attending for my {0}\n", ccm.attendance);
builder.AppendLine();
AppendNonWhiteField(builder, "{0}", ccm.comments);
// initiate the email service and fire off an email
EmailService email = new EmailService();
email.to = ccm.emailTo;
email.from = ccm.emailTo;
email.replyTo = ccm.emailFrom;
email.subject = ccm.messageSubject;
email.messageBody = builder.ToString();
bool sent = email.send();
object result = null;
// Send a system error message via JSON back to the app ifContext
StackExchange Code Review Q#4064, answer score: 6
Revisions (0)
No revisions yet.