Skip to content

Continuous if sentence #658

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

Closed
wants to merge 2 commits into from
Closed

Continuous if sentence #658

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01ea9f6
Continuous if sentence
Aimeast Mar 22, 2014
7d7115c
patch form @nulltoken
Aimeast Apr 5, 2014
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
37 changes: 17 additions & 20 deletions LibGit2Sharp/RepositoryExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,28 +345,23 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
{
singleReturnValue = DereferenceToCommit(repo, identifier as string);
}

if (identifier is ObjectId)
else if (identifier is ObjectId)
{
singleReturnValue = DereferenceToCommit(repo, ((ObjectId) identifier).Sha);
}

if (identifier is Commit)
else if (identifier is Commit)
{
singleReturnValue = ((Commit) identifier).Id;
}

if (identifier is TagAnnotation)
else if (identifier is TagAnnotation)
{
singleReturnValue = DereferenceToCommit(repo, ((TagAnnotation) identifier).Target.Id.Sha);
}

if (identifier is Tag)
else if (identifier is Tag)
{
singleReturnValue = DereferenceToCommit(repo, ((Tag) identifier).Target.Id.Sha);
}

if (identifier is Branch)
else if (identifier is Branch)
{
var branch = (Branch) identifier;
if (branch.Tip != null || !branch.IsCurrentRepositoryHead)
Expand All @@ -375,19 +370,11 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
singleReturnValue = branch.Tip.Id;
}
}

if (identifier is Reference)
else if (identifier is Reference)
{
singleReturnValue = DereferenceToCommit(repo, ((Reference) identifier).CanonicalName);
}

if (singleReturnValue != null)
{
yield return singleReturnValue;
yield break;
}

if (identifier is IEnumerable)
else if (identifier is IEnumerable)
{
foreach (object entry in (IEnumerable)identifier)
{
Expand All @@ -400,6 +387,12 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
yield break;
}

if (singleReturnValue != null)
{
yield return singleReturnValue;
yield break;
}

if (throwIfNotFound)
{
throw new LibGit2SharpException(string.Format(CultureInfo.InvariantCulture, "Unexpected kind of identifier '{0}'.", identifier));
Expand All @@ -415,6 +408,10 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
/// <returns>An <see cref="ObjectId"/> for a commit object.</returns>
internal static ObjectId Committish(this Repository repo, object identifier)
{
while (!(identifier is string) && identifier is IEnumerable)
Copy link
Member

Choose a reason for hiding this comment

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

This loop scares me.

How about this? (Patch to be applied on top of your commit)

diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs
index 90b567f..3cb4e8f 100644
--- a/LibGit2Sharp/RepositoryExtensions.cs
+++ b/LibGit2Sharp/RepositoryExtensions.cs
@@ -374,7 +374,14 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
             {
                 singleReturnValue = DereferenceToCommit(repo, ((Reference) identifier).CanonicalName);
             }
-            else if (identifier is IEnumerable)
+
+            if (singleReturnValue != null)
+            {
+                yield return singleReturnValue;
+                yield break;
+            }
+
+            if (identifier is IEnumerable)
             {
                 foreach (object entry in (IEnumerable)identifier)
                 {
@@ -387,16 +394,11 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
                 yield break;
             }

-            if (singleReturnValue != null)
-            {
-                yield return singleReturnValue;
-                yield break;
-            }
-
             if (throwIfNotFound)
             {
                 throw new LibGit2SharpException(string.Format(CultureInfo.InvariantCulture, "Unexpected kind of identifier '{0}'.", identifier));
             }
+
             yield return null;
         }

@@ -408,10 +410,6 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
         /// <returns>An <see cref="ObjectId"/> for a commit object.</returns>
         internal static ObjectId Committish(this Repository repo, object identifier)
         {
-            while (!(identifier is string) && identifier is IEnumerable)
-            {
-                identifier = ((IEnumerable)identifier).Cast<object>().FirstOrDefault();
-            }
             return repo.Committishes(identifier, true).First();
         }
     }

I think there may exist some way to refactor this further, but I'm not sure how to do it

  • while preserving the "mockability" of types
  • without over-engineering it

@dahlbyk Thoughts?

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 think there may exist some way to refactor this further, but I'm not sure how to do it

something like table driven in CommitFilter ?

Copy link
Member

Choose a reason for hiding this comment

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

I think there may exist some way to refactor this further, but I'm not sure how to do it

something like table driven in CommitFilter ?

I was indeed thinking about some Dictionary<Type, Func> based something, but I'm afraid this would also require a specific IComparer to allow comparison of derived/mocked types...

{
identifier = ((IEnumerable)identifier).Cast<object>().FirstOrDefault();
}
return repo.Committishes(identifier, true).First();
}
}
Expand Down