Methods That Wrap One Method

Question

I think I'm going crazy, someone calms me.

public class MyFile { public static byte[] ReadBinaryFile(string fileName) { return File.ReadAllBytes(fileName); } public static void WriteBinaryFile(string fileName, byte[] fileContents) { File.WriteAllBytes(fileName, fileContents); } } 

People continue to add code, as shown above, to our code base, of course, this is wrong and terrible, and I make the world a favor by deleting it and replacing all (or both in this case ...) links to it with internal code.

Is there any real justification for this kind of thing? Can I skip the big picture? We are pretty YAGNI -centric on our team, and it seems to be flying in the face of this. I could understand if this was the beginning of something more, however this code did not sleep for many months until I stumbled on it today. The more I search, the more I find.

Answer 1

As written, class / methods are garbage. However, I see a situation where a similar pattern can be used legally:

 public interface IFileStorage { byte[] ReadBinaryFile(string fileName); void WriteBinaryFile(string fileName, byte[] fileContents); } public class LocalFileStorage : IFileStorage { ... } public class IsolatedFileStorage : IFileStorage { ... } public class DatabaseFileStorage : IFileStorage { ... } 

In other words, if you want to support different types of storage, then you can actually wrap very simple methods to implement a common abstraction.

As it is written, the class does not implement any interface, and the methods are static, so it is practically useless. If you are trying to support the above template, then refactoring; otherwise get rid of it.

Answer 2

This is pretty stupid until you think about hiding implementation details of these methods.

Take, for example, if you had code like this

 File.WriteAllBytes(fileName, fileContents); 

scattered all over your code, what if some day down you would like to change your file submission method? In this case, you will have to go all over the code and update all these lines of code in order to adopt a new method, where, as in the above version, you only need to change it in one place.

I’m not saying that their path is right, and you’re wrong to fix it, I’m just adding some perspectives

Answer 3

The existence of these methods is disgusting aberration, which should be corrected immediately.

They were probably written by someone who did not know about the File class, and then rewritten by someone who did it, but not as daring as you.

Answer 4

If all the methods do this, take the same list of parameters and skip them, then no, this is pointless, and I think the code is actually becoming less clear. However, if the method also passes default values, except for those that are passed as parameters, or performs some general parameter checking, then this is a more complicated argument.

Are these methods adding placeholders for subsequent logic? If so, you should add comments or even a scary TODO instruction, so that someone turns around later and complete this thought.

Answer 5

I do not think that these methods make sense as static methods of an auxiliary class , but in some cases the template makes sense. For example:

• Separate code from a static class . If MyFile not static, it could serve as an abstraction over the static IO.File object. Code that directly accesses the file system can be very difficult to test, so abstractions like this can be useful. (For example, I have an IFileSystem interface on which all my I / O code depends, and the FileSystem class implements this interface and wraps the main File methods.)

• To maintain consistent levels of abstraction in a method . I don’t like mixing code in the problem area (for example, “customers” and “orders”) with code in the _solution domain) (files, bytes and bits). I often write such wrappers to make the code more readable and give me extensibility points. I would rather read GetDataFileContents() than File.ReadAllBytes("foo.dat") .

• Provide context . If a piece of code is executed for its side effects, such as deleting a text file to delete a sales order, then I prefer to read DeleteCustomerOrderFile("foo.txt") than File.Delete("foo.txt") . The former provides contextual information for the code; the latter does not.

Answer 6

I think the answer to this depends on your team, practice and what the purpose of Ultiamte is for (for example, the code fragment that you found is currently writing to a file, but will write to the web service / database / morse -code after its completion - although this “excuse” is lost by class / method names). I think you yourself answered the question: "We are very focused on YAGNI in our team, and this seems to be flying in the face of this."

The final answer would be to ask the person who wrote it because he wrote it that way.