What do you think about this extension methods package?

[CaptainC]

What do you think about this extension methods package?

Extensions.cs 6.0.800

Extensions.dll contains extension methods that enhance existing C# classes thus making life easier for developers. Extensions.dll supports all versions of.NET from 4.6.2 through 8.0. Highlights are methods such as: -Use.Save() and.Load() for easy state saving of any object in C#...

www.nuget.org

It's well documented and adds a lot of functionality to strings specifically.
Any thoughts?

 

[Skydiver]

Well, right off the bat from the project page, Bigest() extension method is spelled incorrectly. It should be Biggest().

GitHub - cjvandyk/Extensions: Extensions.dll contains extension methods that enhance existing C# classes thus making life easier for developers.

Extensions.dll contains extension methods that enhance existing C# classes thus making life easier for developers. - cjvandyk/Extensions

github.com

And really? The documentation is wrong, and the code implementation is inefficient:

/// <summary>
/// Returns the smallest of two given values.
/// </summary>
/// <param name="val1">The first value to compare.</param>
/// <param name="val2">The second value to compare.</param>
/// <returns>The smallest of the two given values.</returns>
public static int Bigest(int val1, int val2)
{
    if (val1 > val2) return val1;
    if (val2 > val1) return val2;
    return val1;  //val1 == val2
}

 

[Skydiver]

CompoundInterest() is an extension method of double. double should NEVER be used for monetary values due to rounding errors that occur. The extension method should be based on decimal.

 

[Skydiver]

The documentation for ContainsAny() is incorrect where it says it takes IEnumerable of characters, but it only takes an array of characters.

 

[Skydiver]

The ContainsOnly() is implemented inefficiently where it does repeated string replacements and then has this inefficiently written code at the end:

if (remain == "")
{
    return true;
}
return false;

when it could have simply been:

return remain == "";

 

[Skydiver]

The byte [].CopyTo() extension methods don't use the built-in Array.CopyTo() method which can more efficiently perform the copy operations.

 

[Skydiver]

The documentation makes no mention that Process.Elevate() injects extra command line arguments so that it doesn't get into an infinite loop of trying to elevate itself. Also the code doesn't check to see if the current process is already running elevated so that it doesn't have to inject the extra command line arguments to prevent an infinite loop.

 

[Skydiver]

The string.ExceedsLength() extension method takes a ref int, but the top level README documentation says that it can take a constant value.

 

[Skydiver]

To implement the object.Get() extension method, the user needs to use object.Set(string key, object value) extension method which makes sense. Unfortunately, this is completely broken because the implementation just stores the key and value with on kind of reference to the original base object. This completely breaks the principle of "least surprise".

Consider the following code:

class Car
{
    public static string Model { get; set; }
}

:

var gm = new Car() { Model = "GM" };
var saturn = new Car() { Model = "Saturn" };

gm.Set("Year", 1908);
saturn.Set("Year", 1985);

Console.WriteLine(gm.Get("Year"));     // will print out 1985

 

[CaptainC]

Thank you. Lots of really good feedback here.
Anything you liked about it? ;-)

 

[Skydiver]

Really?

/// <summary>
/// For internal use only. Does not waste CPU cycles validating input
/// data or checking for 2 or even numbers.  This method assumes it
/// will always be passed odd numbers greater than 2.
/// Checks if the given number is a prime number.
/// </summary>
/// <param name="number">The given number to check.</param>
/// <param name="INTERNAL_USE_ONLY">Method to distinguish from other overloads.</param>
/// <returns>True if the given number is a prime number, else False.</returns>
public static bool IsPrime(this System.UInt64 number,
                           bool INTERNAL_USE_ONLY)
{
    for (System.UInt64 C = 3; C < (Math.Ceiling(Math.Sqrt(number))); C += 2)
    {
        if ((number % C) == 0)
        {
            return false;
            break;
        }
    }
    return true;
}

Why not just use internal access instead?

Why not implement a more efficient primality check?

 

[Skydiver]

Thank you. Lots of really good feedback here.
Anything you liked about it? ;-)

So far nothing is making me like anything about it other than the joy I'm getting to code review some code that desperately needs some help.

 

[CaptainC]

Really?

/// <summary>
/// For internal use only. Does not waste CPU cycles validating input
/// data or checking for 2 or even numbers.  This method assumes it
/// will always be passed odd numbers greater than 2.
/// Checks if the given number is a prime number.
/// </summary>
/// <param name="number">The given number to check.</param>
/// <param name="INTERNAL_USE_ONLY">Method to distinguish from other overloads.</param>
/// <returns>True if the given number is a prime number, else False.</returns>
public static bool IsPrime(this System.UInt64 number,
                           bool INTERNAL_USE_ONLY)
{
    for (System.UInt64 C = 3; C < (Math.Ceiling(Math.Sqrt(number))); C += 2)
    {
        if ((number % C) == 0)
        {
            return false;
            break;
        }
    }
    return true;
}

Why not just use internal access instead?

Why not implement a more efficient primality check?

Have any suggestions for "a more efficient primality check"?

 

[Skydiver]

Even a simple sieve of Eratosthenes would be more efficient because you'll be doing less division operations. Division is expensive. Yes, for small numbers, a sieve seems to be less efficient, but as the numbers climb up then the cost is amortized. Also considering that you cache your past found primes in a file, you could potentially just cache the sieve state into a file so that you don't always have to start from scratch.

 

[CaptainC]

Even a simple sieve of Eratosthenes would be more efficient because you'll be doing less division operations. Division is expensive. Yes, for small numbers, a sieve seems to be less efficient, but as the numbers climb up then the cost is amortized. Also considering that you cache your past found primes in a file, you could potentially just cache the sieve state into a file so that you don't always have to start from scratch.

Good idea on saving the prime state. Thanks!