Skip to content

Adding an overload to MockFileData that takes an object #394

Closed

Description

@padillah

I did this originally as PR #389 but this was rejected for the following reasons:

Thanks for your work, but I don't think this is something that needs to be in System.IO.Abstractions for the following reasons:

  • the discoverability of the new overload is low, I wouldn't intuitively know what it does
  • this is a low-level library for IO operations that should stay focused
  • it is easy (and IMO more flexible) to do the serialization on top of S.IO.A

I was told to first start an issue if I wanted to contribute. So I will.

My first point would be, the discoverability would be covered by Intellisense the same as any other method in the TestingHelpers library. The PR contained a ///summary block, if that was lacking it can be changed but that's par for discoverability.

Secondly, this is NOT an add to System.IO.Abstractions. It is an add to the TestingHelpers library. Without things like this I am forced to wrap the wrapper. This seriously diminishes the use of TestingHelpers library.

Lastly, I thought the TestingHelpers library WAS on top of S.IO.A. So I'm at a loss to address this point. Is there something I'm not seeing?

Activity

  1. herebebeasties commented on Nov 4, 2018

    @herebebeasties
    Contributor

    FWIW, my view on this is that it's really a very application-specific thing to want to do.

    If you're going to add support for binary object serialization, why isn't there the same thing for XML, JSON, protocol buffers, CSV, YAML, etc?

    They're all arguably equivalent, and arguably used more these days than proprietary .NET serialisation. The reason why not is that this isn't the right layer at which to add such support, as it's really too domain-specific. Yes, it's obviously possible to do it, but it increases the surface area and maintenance effort of the API. If you use binary serialisation a lot it's not exactly hard to add an extension method that does this for you (and that you can use across all IFileSystem implementations).

  2. tathamoddie commented on Nov 4, 2018

    @tathamoddie
    Contributor
    public static class SavedGameTestExtensions
    {
        public static MockFileData AsMockFile(this SavedGame game)
        {
            return new MockFileData({serialisation logic here});
        }
    }
    

    No need to wrap the wrapper, and it's an opportunity for you to make the domain more readable/visible through the app-specific code.

    var savedGame = new SavedGame({something app specific});
    var fileSystem = new MockFileSystem(new Dictionary<string, MockFileData>
    {
        { @"c:\mygame.game", savedGame.AsMockFile() }
    });
    
  3. padillah commented on Nov 5, 2018

    @padillah
    Author

    Thanks for the feedback. I see what was meant now by "this is a low-level library for IO operations that should stay focused". I'm not used to thinking about "everyone that will use this, ever, for any reason".

    This also gives me ideas on how to approach my object serialization.

    Again, appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions