Instantiation in a loop: Verbosity vs. Performance

Question

So in the following bit of code, I really like the verbosity of scenario one, but I would like to know how big of a performance hit this takes when compared to scenario two. Is instantiation in the loop a big deal?

Is the syntactic benefit (which I like, some might not even agree with it being verbose or optimal) worth said performance hit? You can assume that the collections remain reasonably small (N < a few hundred).

// First scenario
var productCategoryModels = new List<ProductCategoryModel>();
foreach (var productCategory in productCategories)
{
    var model = new ProductCategoryModel.ProductCategoryModelConverter(currentContext).Convert(productCategory);
    productCategoryModels.Add(model);
}

// Second scenario
var productCategoryModels = new List<ProductCategoryModel>();
var modelConvert = new ProductCategoryModel.ProductCategoryModelConverter(currentContext);

foreach (var productCategory in productCategories)
{
    var model = modelConvert.Convert(productCategory);
    productCategoryModels.Add(model);
}

Would love to hear your guys' thoughts on this as I see this quite often.

Answer 1

I would approach this questions a bit differently. If whatever happens in new ProductCategoryModel.ProductCategoryModelConverter(currentContext) doesn't change during the loop, I see no reason to include it within the loop. If it is not part of the loop, it shouldn't be in there imo.

If you include it just because it looks better, you force the reader to figure out if it makes a difference or not.

Answer 2

Keep whichever of the formats you're happiest with - if they both perform well enough for your application. Optimize later if you encounter performance problems.

Answer 3

Like Brian, I see no reason to create a new instance if you're not actually changing it - I'm assuming that Convert doesn't change the original object?

I'd recommend using LINQ to avoid the loop entirely though (in terms of your own code):

var modelConverter = new ProductCategoryModelConverter(currentContext);
var models = productCategories.Select(x => modelConverter.Convert(x))
                              .ToList();

In terms of performance, it would depend on what ProductCategoryModelConverter's constructor had to do. Just creating a new object is pretty cheap, in terms of overhead. It's not free, of course - but I wouldn't expect this to cause a bottleneck in most cases. However, I'd say that instantiating it in a loop would imply to the reader that it's necessary; that there was some reason not to use just a single object. A converter certainly sounds like something which will stay immutable as it does its work... so I'd be puzzled by the instantiate-in-a-loop version.