Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###Program

Program

###Program

Program

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

###Program

  • If you use an else if instead of an else you can reduce the indentation of the code by one level.

  • by using short circuit evaluation you will save some time. Replace & with &&. Using && means that the second expression is evaluated only if the first returns true, whereas using & will evaluate always the second regardless ovf the result of the first expression.

  • if you switch the order of if statements to first check premierPublishers.Any(p => p == line) you won't do this that often.

  • the string[] has a Length property to tell you how many items are contained in the array. Using IEnumerable<T>.Count() involves some casts which aren't neccesary.

  • line.Substring(line.Length - 10) why 10 ? Why not 11 or 16. Extract this magic number to a meaningful constant so it is easier to understand.

  • you have releaseItem using camel case casing but you have lineitem which is very hard to read. You should always be consistent in the style you use. In addition, lineitem reffers to a collection of strings, so using the plural form lineItems is more natural.

  • the DateTime.Parse can become dangerous if the returned SubString() won't contain a regular date expression. You should consider to use DateTime.TryParse() instead.

Implementing the mentioned points lead to

const string PremierPublishers = "PREMIER PUBLISHERS";
const string NewReleasesFor = "New Releases For";
ReleaseItem releaseItem = new ReleaseItem();
List<string> premierPublishers = Release.PremierPublishers();
DateTime releaseDate = new DateTime();
const int SomeConstForSubstring = 10;
foreach (var line in Release.GetRelease())
{
 if (line.Contains(NewReleasesFor))
 {
 releaseDate = DateTime.Parse(line.Substring(line.Length - SomeConstForSubstring));
 }
 else if (premierPublishers.Any(p => p == line))
 {
 releaseItem.Publisher = line.Trim();
 }
 else if (!line.Any(p => p.ToString().Contains("\t")))
 {
 releaseItem.Category = line.Trim();
 }
 else
 {
 string[] lineIems = line.Split('\t');
 if (lineIems.Length == 3)
 {
 releaseItem.ItemCode = lineIems[0].Trim();
 releaseItem.Title = lineIems[1].Trim();
 releaseItem.Price = lineIems[2].Trim();
 if (releaseItem.Category != PremierPublishers)
 {
 releaseItem.Publisher = null;
 }
 Release.WriteRelease(releaseDate, releaseItem);
 }
 }
}
lang-cs

AltStyle によって変換されたページ (->オリジナル) /