Skip to content

Fix issues and documentation in Ensure. #884

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
Dec 9, 2014
Merged

Fix issues and documentation in Ensure. #884

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
50 changes: 29 additions & 21 deletions LibGit2Sharp/Core/Ensure.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static void ArgumentNotNullOrEmptyEnumerable<T>(IEnumerable<T> argumentVa
{
ArgumentNotNull(argumentValue, argumentName);

if (argumentValue.Count() == 0)
if (!argumentValue.Any())

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Speed up by doing O(1), not O(n).

{
throw new ArgumentException("Enumerable cannot be empty", argumentName);
}
Expand All @@ -50,7 +50,7 @@ public static void ArgumentNotNullOrEmptyString(string argumentValue, string arg
{
ArgumentNotNull(argumentValue, argumentName);

if (argumentValue.Trim().Length == 0)
if (String.IsNullOrWhiteSpace (argumentValue))

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Use of BCL code to handle this.

{
throw new ArgumentException("String cannot be empty", argumentName);
}
Expand Down Expand Up @@ -145,17 +145,15 @@ public static void ZeroResult(int result)
}

/// <summary>
/// Check that the result of a C call that returns a boolean value
/// was successful
/// Check that the result of a C call returns a boolean value.

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The function actually checked if the result was 0 or 1, not that it passed or anything.

/// <para>
/// The native function is expected to return strictly 0 for
/// success or a negative value in the case of failure.
/// The native function is expected to return strictly 0 or 1.
/// </para>
/// </summary>
/// <param name="result">The result to examine.</param>
public static void BooleanResult(int result)
{
if (result == (int)GitErrorCode.Ok || result == 1)
if (result == 0 || result == 1)
{
return;

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change was made for clarity.

}
Expand All @@ -164,11 +162,11 @@ public static void BooleanResult(int result)
}

/// <summary>
/// Check that the result of a C call that returns an integer value
/// was successful
/// Check that the result of a C call that returns an integer
/// value was successful.
/// <para>
/// The native function is expected to return strictly 0 for
/// success or a negative value in the case of failure.
/// The native function is expected to return 0 or a positive

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed up documentation to state that positive values are also successful.

/// value for success or a negative value in the case of failure.
/// </para>
/// </summary>
/// <param name="result">The result to examine.</param>
Expand All @@ -182,16 +180,6 @@ public static void Int32Result(int result)
HandleError(result);
}

public static void NotNullResult(Object result)
{
if (result != null)
{
return;
}

HandleError((int)GitErrorCode.Error);
}

/// <summary>
/// Checks an argument by applying provided checker.
/// </summary>

@Therzok Therzok Dec 8, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not used anywhere, and it looks like it's not something we want? I thought it would be best to remove.

Expand All @@ -208,6 +196,15 @@ public static void ArgumentConformsTo<T>(T argumentValue, Func<T, bool> checker,
throw new ArgumentException(argumentName);
}

/// <summary>
/// Check that the result of a C call that returns a non-null GitObject
/// using the default exception builder.
/// <para>
/// The native function is expected to return a valid object value.
/// </para>
/// </summary>
/// <param name="gitObject">The <see cref="GitObject"/> to examine.</param>
/// <param name="identifier">The <see cref="GitObject"/> identifier to examine.</param>
public static void GitObjectIsNotNull(GitObject gitObject, string identifier)
{
Func<string, LibGit2SharpException> exceptionBuilder;
Expand All @@ -224,6 +221,17 @@ public static void GitObjectIsNotNull(GitObject gitObject, string identifier)
GitObjectIsNotNull(gitObject, identifier, exceptionBuilder);
}


/// <summary>
/// Check that the result of a C call that returns a non-null GitObject
/// using the default exception builder.
/// <para>
/// The native function is expected to return a valid object value.
/// </para>
/// </summary>
/// <param name="gitObject">The <see cref="GitObject"/> to examine.</param>
/// <param name="identifier">The <see cref="GitObject"/> identifier to examine.</param>
/// <param name="exceptionBuilder">The builder which constructs an <see cref="LibGit2SharpException"/> from a message.</param>
public static void GitObjectIsNotNull(
GitObject gitObject,
string identifier,
Expand Down