Skip to content

Fix issues reported by CodeAnalysis (NRefactory). #734

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

Merged
nulltoken merged 1 commit into from
May 31, 2014
Merged

Fix issues reported by CodeAnalysis (NRefactory). #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 2 additions & 7 deletions LibGit2Sharp/ContentChanges.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace LibGit2Sharp
public class ContentChanges
{
private readonly StringBuilder patchBuilder = new StringBuilder();
private bool isBinaryComparison;

/// <summary>
/// Needed for mocking purposes.
Expand All @@ -31,7 +30,7 @@ internal ContentChanges(Repository repo, Blob oldBlob, Blob newBlob, GitDiffOpti

internal ContentChanges(bool isBinaryComparison)
{
this.isBinaryComparison = isBinaryComparison;
this.IsBinaryComparison = isBinaryComparison;
}

internal void AppendToPatch(string patch)
Expand Down Expand Up @@ -60,11 +59,7 @@ public virtual string Patch
/// <summary>
/// Determines if at least one side of the comparison holds binary content.
/// </summary>
public virtual bool IsBinaryComparison
{
get { return isBinaryComparison; }
private set { isBinaryComparison = value; }
}
public virtual bool IsBinaryComparison { get; private set; }

private int FileCallback(GitDiffDelta delta, float progress, IntPtr payload)
{
Expand Down
10 changes: 4 additions & 6 deletions LibGit2Sharp/NoteCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,9 @@ internal IEnumerable<string> NamespaceRefs
{
get
{
return new[] { NormalizeToCanonicalName(DefaultNamespace) }.Concat(
from reference in repo.Refs
select reference.CanonicalName into refCanonical
where refCanonical.StartsWith(Reference.NotePrefix, StringComparison.Ordinal) && refCanonical != NormalizeToCanonicalName(DefaultNamespace)
select refCanonical);
return new[] { NormalizeToCanonicalName(DefaultNamespace) }.Concat(repo.Refs
Copy link
Member Author

@Therzok Therzok May 31, 2014

Choose a reason for hiding this comment

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

Replaced really long LINQ with really readable linq extension.

Copy link
Member Author

@Therzok Therzok May 31, 2014

Choose a reason for hiding this comment

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

Also, this will perform better. The previous has 3 x O(n) passes. (Select, Where, Select).

The second version only does 2x passes, (Select, Where).

That's a down-side of real LINQ. Identity is resolved in O(n) pass not as a NOP.

Copy link
Member

@dahlbyk dahlbyk Jun 2, 2014

Choose a reason for hiding this comment

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

Already merged, so pedantic at this point, but the original query and your rewrite should generate identical extension method calls behind the scenes. At least according to the spec (C# 3 §7.15.2.5), "degenerate query expressions" (that trivially select the elements of the source) only result in an "extra" step if there's nothing else interesting about the query, and that's only to "ensure that the result of a query expression is never the source object itself".

As generated by dotPeek at e69b6e0:

return Enumerable.Concat<string>((IEnumerable<string>) new string[1]
  {
    NoteCollection.NormalizeToCanonicalName(this.DefaultNamespace)
  },
  Enumerable.Where<string>(
    Enumerable.Select<Reference, string>(
      (IEnumerable<Reference>) this.repo.Refs,
      (Func<Reference, string>) (reference => reference.CanonicalName)
      ),
    (Func<string, bool>) (refCanonical =>
      {
        if (refCanonical.StartsWith(Reference.NotePrefix, StringComparison.Ordinal))
          return refCanonical != NoteCollection.NormalizeToCanonicalName(this.DefaultNamespace);
        else
          return false;
      })
    )
  );

It's also important to note that chains of Select, Where and other streamed LINQ methods don't result in multiple passes through the sequence, but rather a single pass through effectively nested enumerators.

Copy link
Member Author

@Therzok Therzok Jun 2, 2014

Choose a reason for hiding this comment

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

Still more readable. :D

.Select(reference => reference.CanonicalName)
.Where(refCanonical => refCanonical.StartsWith(Reference.NotePrefix, StringComparison.Ordinal) && refCanonical != NormalizeToCanonicalName(DefaultNamespace)));
}
}

Expand Down Expand Up @@ -155,7 +153,7 @@ internal static string NormalizeToCanonicalName(string name)
return string.Concat(Reference.NotePrefix, name);
}

internal string UnCanonicalizeName(string name)
internal static string UnCanonicalizeName(string name)
{
Ensure.ArgumentNotNullOrEmptyString(name, "name");

Expand Down