1

I initially wanted to quickly select certain records (using a Linq qry) which match a default date condition (01/01/0001) - 

var defDates = (from rec in myRecords where rec.myDate == System.DateTime.MinValue select rec.myDate);

and then update those record(s) to a Null value. However, since those returned records have only a get, it means they are read-only (makes sense since I'm using Linq).

So the best I could come up with is a c# foreach which will catch default dates and sets to Null (i.e. I'm exporting to Excel via Aspose.cells, so I need those cell values to just be blank):

foreach (myExportClass rec in myRecords)
{
    if (rec.myDate == System.DateTime.MinValue)
    {
        rec.myDate = null;
    }
};

Okay, this works, and exports the required Null value(s) to Excel (a blank cell, actually).

However, the question is: could I find a more efficient way of handling this default date scenario ? Seems to me that a foreach() may waste resources if there are several thousand records coming back.

Fyi: the records are coming back from the backend this way, so I need to deal with this in c#.

thanks in advance

bob.mazzo
  • 5,183
  • 23
  • 80
  • 149
  • 1
    Are you having an actual problem with runtime performance? At any rate, I don't think there's a special high-speed looping construct in .NET. – 15ee8f99-57ff-4f92-890c-b56153 Nov 10 '17 at 20:43
  • 2
    What resources are you expecting a `foreach` to consume that aren't actually needed for your situation? – Servy Nov 10 '17 at 20:44
  • Might want to look into `Parallel.ForEach` – BurnsBA Nov 10 '17 at 20:47
  • 3
    Two questions about the `foreach` you propose: 1) where is `myObj` comming from? is the looping variable? in that case you should rename it to `rec` or viceversa. 2) you break (i.e. exit the loop) after the first default date is found, is that correct? That way you only "fix" the first one... – Tao Gómez Gil Nov 10 '17 at 20:48
  • 1
    Why the `break;`? it means you only change the first value you find to null... I don't think you can get much more efficient then this, but you can use the `ForEach` extension method on your `List` with a lambda to make it shorter: `myRecords.ForEach(r => {if(r.MyDate == DateTime.MinValue) r.MyDate = null;});` – Zohar Peled Nov 10 '17 at 20:49
  • 2
    @BurnsBA Unless `myDate` is some expensive to compute property getter, that'd almost certainly be slower. – Servy Nov 10 '17 at 20:50
  • 2
    @ZoharPeled What problems are you solving by doing that? The code isn't even any shorter than the code in the question, nor does it perform better. – Servy Nov 10 '17 at 20:52
  • @servy I don't follow. Enumerate several thousand records sequentially and check & update them one at a time, or enumerate them in parallel and check & update them that way. – BurnsBA Nov 10 '17 at 20:55
  • 1
    i'd use default(DateTime) instead of DateTime.MinValue – Dave Black Nov 10 '17 at 20:55
  • 1
    @Servy It's not solving any problem. As I wrote it's not going to be more efficient, just a bit shorter - a single line instead of 3 (not counting the `{` and `}`. That's why it's a comment and not an attempted answer. – Zohar Peled Nov 10 '17 at 20:56
  • 1
    @BurnsBA - because creating and destroying threads is not free (to the CPU / application). If the operation is simple enough that it can be accomplished faster than the time it takes to create threads, you can actually see a performance **decrease** using `Parallel.ForEach` due to the added overhead of thread management. - https://stackoverflow.com/a/17486002/130387 – Tommy Nov 10 '17 at 20:56
  • 3
    @BurnsBA The overhead you have from managing the threads is going to be *significantly* more expensive than the single date equality comparison (and infrequently, also an assignment). And the enumeration of the sequence is going to be done serially in either case (as the underlying collection can only be accessed from a single thread safely). Unless the actual work being done concurrently is expensive (which isn't the case here unless `myDate` is some expensive property getter) that will take more time, not less. – Servy Nov 10 '17 at 20:58
  • @ZoharPeled If you want to remove the line breaks, then you can just remove the line breaks. You don't need to call another method that doesn't do anything productive in order to remove some unnecessary whitespace from your code. – Servy Nov 10 '17 at 20:59
  • 2
    Just leave your foreach as is. Looping over several thousands items costs nothing compared to reading them from your backend, and you don't need any improvement in that part (and your linq query above performs the same foreach). If you ever find problems with performance - I bet they will not be here. – Evk Nov 10 '17 at 21:00
  • To all, I changed myObj to rec within the foreach (). I only used the break for one test case, however I removed it since I need to check all records. – bob.mazzo Nov 10 '17 at 21:22
  • What is the size of the collection, do have any approximate values ? – vasil oreshenski Nov 10 '17 at 21:28
  • Thank you for the helpful comments guys. The lead dev on my team does code reviews, and is very critical - so I wanted to be a step ahead by attempting to write the best code. I.e. I was concerned that I would loop thousands of records unnecessarily if there was a quicker way. – bob.mazzo Nov 10 '17 at 21:28
  • Keep in mind that even iterating a million sized collection in relatively old processor will take no more then 5 - 15 ms. This are really rough number ... everything is relative of of course (cpu, ram .. etc), but just keep in mind that iterating si not slow at all. – vasil oreshenski Nov 10 '17 at 21:31
  • @vasil, it appears not very large. I only see a couple hundred recs at the most. I do need to confirm the max collection size with the DB folks, however. – bob.mazzo Nov 10 '17 at 21:32
  • Have you tried using PLinq? collection.Where(test => test.Time <= DateTime.MinValue).AsParallel().ForAll(test => test.Time = null) – Ferdinand Brunauer Nov 10 '17 at 21:33
  • If it is couple of hundred recs, don't worry about it. You've done it the right way ;) – vasil oreshenski Nov 10 '17 at 21:34
  • 1
    couldn't you just use a `select new myExportClass { myDate = null};` in the linq query and avoid the need to loop over the list? – Sorceri Nov 10 '17 at 21:45
  • @FerdinandBrunauer That's going to be noticeably slower than a regular foreach, for reason already mentioned in earlier comments. – Servy Nov 10 '17 at 22:59
  • @Sorceri That is a different behavior; it's not a functionally equivalent translation. Additionally, constructing a new value for each item is *going* to be more expensive than the assignment. And you're iterating over the collection in either case, whether you iterate over it to create a new value or to mutate the existing value doesn't change that you're iterating over it. – Servy Nov 10 '17 at 23:00

0 Answers0