10

Say I re-implement a method for finding the position of a string within a string. So I get either the position which is an integer, or a "magic number" like -1. I see this pattern in so many programming languages, where some return a value like null, some return some random "out of range" value like -`, some totally throw an error.

Is there a "best practice" on this or at least something that's a bit more explicit in other programming language that I'm not aware of?

  • 11
    This is not fully language agnostic. At least it depends on if you are using a language providing exceptions or not. It also depends on the idioms which are established in the specific language environment. – Doc Brown Jan 31 '17 at 14:20
  • 3
    @gnat I don't think it's a dupe: the other question is specifically about C# but the OP states his question is language agnostic (and maybe he/she isn't a C# dev). If anything, since this question cannot be answered in general (because not all alternatives are available for all languages), maybe it should be closed as "too broad" instead. – Andres F. Jan 31 '17 at 14:54
  • @AndresF. you have a point about "too broad", I agree. Why is asking a question on “best practice” a bad thing? – gnat Jan 31 '17 at 15:56
  • @AndresF.: the fact the OP thinks his question is language agnostic does not mean he is right. – Doc Brown Jan 31 '17 at 20:00
  • 1
    @DocBrown Agreed. In that case, the right closure reason is "too broad", not "dupe of a C# question" (because the question isn't about C#, and answers for C# don't necessarily apply to other languages). – Andres F. Jan 31 '17 at 20:12
21

Follow the principle of least surprise for whatever language you are coding in. People are going to, at some level, expect your function to follow the paradigms and standards of your language. So if your language typically uses error codes like -1 on search functions (like C, C++, etc.), do that. If you're in a functional language where an option type is commonplace, do that. If your language typically uses nulls for that sort of thing, do that. Etc, etc. Make your function easy to use for the people that will be using it. They are far more likely to get it right when it works like other things they are familiar with.

  • 2
    I agree, but "Sic transit gloria mundi: - this is how things stay the same. Maybe they should stay the same? Maybe we don't need so many new languages and approaches and metaphors? Just stick with what works. Long live C! – user251748 Jan 31 '17 at 16:42
  • 3
    This is really the only sensible answer. There is no "one size fits all" that is appropriate for all languages and environments. – Polygnome Jan 31 '17 at 16:51
15

A popular way, amongst functional languages at least, is to use a Maybe type (or Option in some functional languages).

Maybe<int> FindPosition(string stringToSearch, char charToFind)
    // Return Some(int) if found or none if not.

The function then will yield an int value of the position if found, and none if nothing was found.

This avoids the problems inherent in using a struct or tuple to return a flag indicating success, and a posibly undefined int value depending on that flags state; throwing an exception; using a magic number etc.

  • 1
    In some languages Nullable{Int} (Julia, C#) – Lyndon White Jan 31 '17 at 15:46
  • 1
    @LyndonWhite, There are plenty of C# libraries that support Maybe<T>, such as my own SuccincT library. But it does have Nullable<T> built in as syntactic sugar, so your point is well made. – David Arno Jan 31 '17 at 16:15
  • 1
    Nulls were a complication added to SQL, and they are a complication here also. Use with care. – user251748 Jan 31 '17 at 16:42
  • @DavidArno I knew C# had access to Maybe (or Option) via the F# libraries. I did not know it was in common use these days. I always thought Nullable was the C# metaphor, but I'm now thinking i did not think enough. Interesting. Julia definately uses it's Nullable just like Maybe or Option – Lyndon White Feb 1 '17 at 0:27
  • The Nullable type in C# is a limited option type; it can only be used with value types (not reference types) that are not already Nullable. So it's not legal to do Nullable<string>, since string is a reference type, or to do Nullable<Nullable<int>>. – Tanner Swett Jun 21 '17 at 12:59
3

Sometimes a TryGet Pattern can be useful in the scenario that you describe.

Example:

public bool TryGetValue(string input, string toFind, out int value)

Note: This is also telling you that no exceptions should come out of this method as well for languages that support exceptions.

  • 3
    This precludes a possibility of making the result const in the caller like const auto result = getMyResult();. (although it wasn't me who downvoted) – Ruslan Jan 31 '17 at 16:12
  • 1
    I always thought that the "Try" aspect was implicit in the fact that this is a program, not a rack falling to the ground, but someone took Exception to that. What Would Yoda Say? – user251748 Jan 31 '17 at 16:45
2

There are lots of approaches to this and while I do agree with Becuzz to some degree that you should usually follow the conventions of the language/team, it's not always completely cut-and-dried. In java, there are competing ideas about the best way to approach this kind of thing and what the right answer is depends on the context of the method.

One thing that I would first try to disabuse you of is that returning -1 is a 'random' value or magic number. You really should not be coding to the specific value. In that case, you should check for values less than 0 for not found. I've seen assumptions like that lead to bugs. For example, in Java Integer.compare is implemented using subtraction. That means pretty much any int (positive or negative) may be returned but I've seen code that assumes compare calls will only return -1, 0, or 1. Another case in point is Arrays.binarySearch which, when an element is not found returns a negative number indicating where the element would be in the order if it were in the array.

These kinds of approaches are useful because they return more information that simply "not found". There are other cases though different approaches are common. For example, a method meant to retrieve a collection of items meeting a certain criteria could return a null. Another option is to return an empty collection. In most cases, the latter is preferable because you don't have to worry about null-pointer checks and there are usually no checks required if all you want to do is iterate over the results. Returning null is common and expected in Java but it's also problematic. Another option would be to throw an exception but that is generally discouraged because of the overhead of creating a stack trace which is usually overstated but real.

The point is You don't need to blindly follow convention if you can achieve superior results. But in lieu of that, do what people expect.

1

One way is to return a 2-field struct by value, in C++ this typically looks like this:

struct find_result
{
    bool found;
    size_t index;
};

Then the callee can set found to true or false, only set index when found is true, and document that index can only be used if found is true.

The C++ standard library uses this with std::pair<>, where first is a bool and second carries the context (like index does above).

  • Also, in languages with multiple return values like lua or go, it often is the case that you return multiple values, one of them indicating success and the other(s) giving you the result(s) (if any). – hoffmale Jan 31 '17 at 13:55
  • 3
    This does not prevent returning garbage. What value will you assign to index if the substring is not found? Or just return it uninitialised? The first option does not solve the original problem, the second is even worse. And the fact that "the programmer should not use index if found is false" means nothing as there is nothing that actually prevents the programmer from using this value even if it is "random" and should not be used. Repeat after me: we do not mix return values and error messages in the same channel. – Mael Jan 31 '17 at 14:08
  • 2
    @Mael I agree that return values and error messages should not be mixed together in the same channel. However, I disagree that such a thing is happening in this example. If I search for something I am asking two questions: "Was is found?" and "If so, where?". In other words, not finding something is not an error when searching; it's one of the expected values. Now running off the end of a null-terminated string, for example, because it was not properly null-terminated is an error and could be handled via an exception or a Result<T> type etc. – Altainia Jan 31 '17 at 14:32
  • 4
    Better yet use an already invented wheel, like boost::optional. – Ruslan Jan 31 '17 at 16:10
1

In this specific case a valid return value (indicating a match was found) will be from zero up, while -1 indicates nothing was found. You are left with all the other negative values to indicate there was a problem. So specifically here I would just return -2 to indicate there was an error.

As Johann said above you could use complex structures to return 'status' type values but I would only use that for APIs where complex return values are required and it is necessary to indicate if the actual result returned was 'real'. So in your specific case if it were possible to return valid negative values then you do not have the option of using the negative numbers to indicate state and so returning a pair of values would be useful.

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