Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to projections tests in Introduction to Event Sourcing workshop #128

Merged
merged 1 commit into from Mar 15, 2022

Conversation

oskardudycz
Copy link
Owner

@oskardudycz oskardudycz commented Mar 15, 2022

  1. Fixed Database.Store method to do upsert instead of add.
  2. Added hint on how to register an event handler.
  3. Fixed copy/paste issues in projections tests

@oskardudycz oskardudycz force-pushed the workshop/introduction-to-event-sourcing branch from fc71a3d to a2f9e21 Compare March 15, 2022 11:06
@oskardudycz oskardudycz merged commit 819905d into main Mar 15, 2022
@oskardudycz oskardudycz deleted the workshop/introduction-to-event-sourcing branch March 15, 2022 11:10
if (!storage.ContainsKey(id))
storage.Add(id, obj);
else
storage[id] = obj;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idiomatic impl for upsert given the Dictionary API is to do only this line, blindly
(the ContainsKey is waste)
(aside: ContainsKey is not present in the ConcurrentDictionary API either for the more obvious reason of it being prone to race conditions)

(In F#, Map.add has this upsert semantic)

@@ -12,6 +12,12 @@ public class Database
var validFrom = DateTime.UtcNow.AddMilliseconds(random.Next(250, 1000));

storage.Add(id, new DataWrapper(obj, validFrom));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you meant to remove this (but again should be L20 only)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants