patterncsharpModerate
Is there a more readable way to write this add to a list only if the element not exists? (perfectionistic)
Viewed 0 times
thisreadabletheperfectionisticmorewaywriteelementexistslist
Problem
Take a timer in your hands, then have a look to the following code. What is it doing?
I took 52 seconds to realize more or less what it does..
It compares a list called categories with an actual element. If it already exist in the list it takes it and stores to the category variable. Otherwise it creates this new element, stores it into the category variable and also add it to the list.
Mmm... very not clear to me. I strongly prefer the second piece of code.
What do you think? How would you rewrite the code to make it more readable?
Do you find an utility in generalizing patterns like this one and creating classes and framework for handling this behaviors in a standardized way?
I took 52 seconds to realize more or less what it does..
It compares a list called categories with an actual element. If it already exist in the list it takes it and stores to the category variable. Otherwise it creates this new element, stores it into the category variable and also add it to the list.
Mmm... very not clear to me. I strongly prefer the second piece of code.
What do you think? How would you rewrite the code to make it more readable?
Do you find an utility in generalizing patterns like this one and creating classes and framework for handling this behaviors in a standardized way?
foreach (var summaryOdd in odds)
{
var category = new PlacedCategory();
var eventId = summaryOdd.Championship;
if (categories.Any(cat => cat.Name == eventId))
{
category = categories.First(cat => cat.Name == eventId);
}
else
{
category.Name = eventId;
categories.Add(category);
}foreach (var summaryOdd in odds)
{
var category = new PlacedCategory();
var eventId = summaryOdd.Championship;
category = FindOrCreateCategoryByEventId(categories, eventId);Solution
I don't think it is necessary to create a method; the code seems atomic enough (SRP).
But I would refactor the code to make it clearer, and to avoid enumerating the categories twice:
But I would refactor the code to make it clearer, and to avoid enumerating the categories twice:
foreach (var summaryOdd in odds)
{
string eventId = summaryOdd.Championship;
var category = categories.FirstOrDefault(cat => cat.Name == eventId);
if(category == null)
{
category = new PlacedCategory
{
Name = eventId,
};
categories.Add(category);
}
}Code Snippets
foreach (var summaryOdd in odds)
{
string eventId = summaryOdd.Championship;
var category = categories.FirstOrDefault(cat => cat.Name == eventId);
if(category == null)
{
category = new PlacedCategory
{
Name = eventId,
};
categories.Add(category);
}
}Context
StackExchange Code Review Q#64237, answer score: 10
Revisions (0)
No revisions yet.