Set internal modifier on extension methods on object CloudinaryDotNet.Core.SystemExtension

Open
  • Is something wrong?
#387 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
48/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
csharp
Domain
api

Research direction

Locate CloudinaryDotNet.Core.SystemExtension and inspect its usages to confirm Clone and CopyPropertiesTo are only needed internally. Change the exposed API as requested, then build the SDK and run its available tests to verify internal callers still work and the extension methods are no longer publicly usable.

Written by the indexing model from the issue text.

Description

Removing of a "feature" request for Cloudinary .NET SDK

CloudinaryDotNet.Core add extensions methods to object base type, which might be used by the library itselve, but shouldn't be public and usable on a project using this library.
The two following extension methods must be internal only:

namespace CloudinaryDotNet.Core;

public static class SystemExtension
{
  /// <summary>Clones the object.</summary>
  /// <param name="source">The source to clone.</param>
  /// <typeparam name="T">Object type.</typeparam>
  /// <returns>New instance of the cloned object.</returns>
  public static T Clone<T>(this T source)
  {
    return JsonConvert.DeserializeObject<T>(JsonConvert.SerializeObject((object) source));
  }

  /// <summary>
  /// Copies the writable properties from the source object to the destination object.
  /// </summary>
  /// <param name="source">The source object whose properties will be copied.</param>
  /// <param name="destination">The destination object where the properties will be copied.</param>
  public static void CopyPropertiesTo(this object source, object destination)
  {
    foreach (PropertyInfo runtimeProperty in typeof (FileDescription).GetRuntimeProperties())
    {
      if (runtimeProperty.CanWrite)
      {
        object obj = runtimeProperty.GetValue(source);
        runtimeProperty.SetValue(destination, obj);
      }
    }
  }
}

Explain your use case

I shouldn't be able to use these methods on any object of my domain, system object, etc.
For example, this code is "syntaxically" ok, but absolutely not in execution:

        System.Net.Http.SocketsHttpHandler handler = new System.Net.Http.SocketsHttpHandler();
        handler.Clone();

Describe the problem you’re trying to solve

From my point of view, this is the responsibility of each projects to add the extension methods they need on base system type.
When I use a library to simplify access to cloudinary API, i don't expect to get some system type helper methods, which:

  1. will not works as i expect regarding they naming. Each people will have a different perception of what a "Clone" method do, maybe from your use case, Json IN/OUT is enough, but for another person, it will not be the case.
  2. will create a dependence on the library outside of the desired scope. (CloudinaryDotNet.Core would be used only on a "CdnConnectorService", but might be used everywhere because of that. )

Do you have a proposed solution?

Set these methods / extension class as internal instead of public.

(thanks for your work on the library btw! :) )

Dominant language
C#
Stars
105
Forks
72
No merged PRs in 30d

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cloudinary/CloudinaryDotNet

All issues in cloudinary/CloudinaryDotNet

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.