I'm trying to think of the cleanest way to implement a couple of methods that open a file.

Consider the following method signatures:

public static DomainObject Load(Uri urlToFile)
    /* downloads file and calls Load(savedToFilename) */

public static DomainObject Load(string filename) 
    /* creates filestream and calls Load(stream) */

public static DomainObject Load(Stream stream) 
    /* does actual loading */ 

I'm trying to implement some concepts from Clean Code, specifically:

When constructors are overloaded, use static factory methods with names that describe the arguments. For example,

Complex fulcrumPoint = Complex.FromRealNumber(23.0);

is generally better than

Complex fulcrumPoint = new Complex(23.0);

Now, I know I do not have overloaded constructors, per se (I refactored away from that), but I think the principle is the same. So that implies that my Load methods be refactored to something like:

public static DomainObject FromURI(Uri urlToFile);
public static DomainObject FromFile(string filename);
public static DomainObject FromStream(Stream stream);

But, tbh, I think it's more intuitive to use what I've already got. From a consumer's perspective it feels like Open will take whatever source I happen to have whereas the other method requires me to think first about what my source is and then wonder if there is a specific method for that.

So I ask, from your more experienced viewpoint, which is better and why?

  • 3
    I would consider Clean Codes method bloat. I can see that 23 is a real number; why do I need the method name to say that? Oct 11 '10 at 0:25
  • I agree with mathepic that it does seem like code bloat. I would say it may be valid if you work in a language which doesn't have a de facto IDE (i.e. PHP vs. C# & Visual Studio), or if the IDE doesn't support intellisense. However, if intellisense is supported or offered, then I'm typically OK with overloaded factory methods, since they can indicate what the type of the expected parameter(s) and their names should be representative of their use.
    – Hugo
    Oct 11 '10 at 2:41

I think "Load" is the way to go for a few reasons:

  • The parameter type is already in the parameter list- why specify it again in the method name?
  • If you happen to have multiple methods that produce something a stream in a class, "FromStream" becomes a problem.
  • In the age of intellisense, it's much more logical to type "Load" and get a list of possible parameter options, rather than look at three different methods (especially if more methods than just those in this set happen to start with "From").
  • Another decent rule of thumb is "name methods after verbs when you can," which votes in favor of something akin to "Load," or at least "LoadFrom..."
  • Totally! And not to mention, it's so much cooler to say "method overloads".
    – kowsheek
    Oct 11 '10 at 6:07
  • +1 for good answer. Also, all methods exhibit the same behaviour and return the same result; as such, it's better to overload the method as it makes this point clear to the programmer (you, in this case). If you use slightly different names, the programmer might get confused. Finally, "FromXYZ" doesn't tell you what the method does, but "Load" does (well, maybe it could be named even better, but it's vastly better than "FromXYZ" at least). Always strive for self-commenting code.
    – gablin
    Oct 11 '10 at 6:50
  • @gablin: The name FromXYZ tells me exactly what the method should do: construct a new instance using information contained in an XYZ. By contrast, Load does not indicate whether it should modify an existing instance by replacing its contents with data from the indicated source, combine its contents with the source, or perhaps create a new object which has some properties of the existing object and incorporates data from the indicated source.
    – supercat
    Aug 28 '14 at 1:19
  • @supercat: I'd agree that Load on its own is a bit vague, but loadFrom should convey the same semantics as FromXYZ whilst avoiding the name repetition which is already part of the function argument. Moreover, the information of whether the input arguments are modified or not should be made clear through the use of const.
    – gablin
    Aug 29 '14 at 13:35
  • @gablin: In many cases, it's possible for a single type to be capable of holding many kinds of things. For example, a string may hold either the name of a local text file, a URL identifying a remote text file, or a quantity of text which should be interpreted as the contents of such a file. While it would be possible to require that the string be an URL, I think fromFile(theName) is cleaner than loadFrom(URLFormatter.fileNameToURL(theName)). Also, the question wouldn't be whether the input arguments are modified, but rather...
    – supercat
    Aug 29 '14 at 15:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.