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.

Sign up for GitHub

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

Update AutoClone.md #328

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Update AutoClone.md #328

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
949f7fe
Update AutoClone.md
oasaleh Nov 1, 2024
c3b148d
Update AutoClone.md
jodydonetti Nov 27, 2024
b01b3e8
Merge branch 'main' into patch-1
jodydonetti Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/AutoClone.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,28 @@ cache.SetupSerializer(new FusionCacheSystemTextJsonSerializer());

cache.Set("foo", new Person { Name = "John" });

// RETURNS A CLONE OF CACHED INSTANCE (ORIGINAL REMAINS UNCHANGED)
// RETURNS A CLONE OF THE CACHED INSTANCE
var person1 = cache.GetOrDefault<Person>("foo", options => options.SetAutoClone(true));
Console.WriteLine($"person1: {person1.Name}");
Console.WriteLine();

// RETURNS A CLONE OF CACHED INSTANCE (MODIFICATIONS AFFECT ONLY THE CLONE)
// RETURNS A CLONE OF THE CACHED INSTANCE: CHANGES APPLIED ONLY THE CLONE, CACHED INSTANCE REMAINS UNCHANGED
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed a couple of points regarding this comment:

  1. The phrase “applied only” should be updated to “applied only to” for grammatical correctness.
  2. The comment feels slightly verbose, which makes it less clear. For example, the term “clone” is used without pairing it with “instance”. Using “cloned instance” would improve clarity and consistency. I think reverting it back to (MODIFICATIONS AFFECT ONLY THE CLONE) would be ideal. However, this version could also work:
    // RETURNS A CLONED INSTANCE OF THE CACHED OBJECT: CHANGES ARE APPLIED ONLY TO THE CLONED INSTANCE, LEAVING THE CACHED INSTANCE UNMODIFIED

var person2 = cache.GetOrDefault<Person>("foo", options => options.SetAutoClone(true));
person2.Name = "Jane";
Console.WriteLine($"person1: {person1.Name}");
Console.WriteLine($"person2: {person2.Name}");
Console.WriteLine();

// RETURNS DIRECT REFERENCE TO CACHED INSTANCE (MODIFICATIONS AFFECT THE CACHE)
// RETURNS DIRECT REFERENCE TO THE CACHED INSTANCE: CHANGES APPLIED TO THE CACHED INSTANCE ITSELF
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as the comment above. Also, it's missing the verb "are" in "Changes are applied to", which makes it verbose. 🤷

var person3 = cache.GetOrDefault<Person>("foo");
person3.Name = "Jim";
Console.WriteLine($"person1: {person1.Name}");
Console.WriteLine($"person2: {person2.Name}");
Console.WriteLine($"person3: {person3.Name}");
Console.WriteLine();

// RETURNS SAME REFERENCE TO CACHED INSTANCE
// MODIFICATIONS AFFECT BOTH person3 AND person4 AS THEY SHARE THE CACHED REFERENCE
// RETURNS DIRECT REFERENCE TO THE CACHED INSTANCE: THE INSTANCE IS THE SAME AS BEFORE
// CHANGES APPLIED TO BOTH person3 AND person4 AS THEY POINT TO THE SAME CACHED INSTANCE
var person4 = cache.GetOrDefault<Person>("foo");
person4.Name = "Joe";

Expand Down