45

I have written a struct that represents latitude/longitude coordinates. Their values range from -180 to 180 for longtitudes and 90 to -90 for lattitudes.

If a user of that struct gives me a value outside of that range, I have 2 options:

  1. Throw an exception (arg out of range)
  2. Convert the value to the constraint

Because a coordinate of -185 has meaning (it can very easily be converted to +175 as those are polar coordinates), I could accept it and convert it.

Is it better to throw an exception to tell the user that his code has given me a value that it shouldn't have?

Edit: Also I know the difference between lat/lng and coordinates, but I wanted to simplify that for easier discussion - it wasn't the brightest of ideas

RaidenF
  • 1,579

8 Answers8

53

If the core of your question is this...

If some client code passes an argument whose value is invalid for the thing that my data structure is modeling, should I reject the value or convert it to something sensible?

...then my general answer would be "reject", because this will help draw attention to potential bugs in the client code that are actually causing the invalid value to appear in the program and reach your constructor. Drawing attention to bugs is generally a desired property in most systems, at least during development (unless it's a desired property of your system to muddle through in case of errors).

The question is whether you're actually facing that case.

  • If your data structure is intended to model polar coordinates in general, then accept the value because angles out of the -180 and +180 range aren't really invalid. They are perfectly valid and they just happen to always have an equivalent that's within the range of -180 and +180 (and if you want to convert them to target that range, feel free - the client code doesn't usually need to care).

  • If your data structure is explicitly modeling Web Mercator coordinates (according to the question in its initial form), then it's best to follow any provisions mentioned in the specification (which I don't know, so I won't say anything about it). If the specification of the thing you're modeling says that some values are invalid, reject them. If it says that they can be interpreted as something sensible (and thus they're actually valid), accept them.

The mechanism you use to signal whether the values were accepted or not depends on the features of your language, its general philosophy and your performance requirements. So, you could be throwing an exception (in the constructor) or returning a nullable version of your struct (through a static method that invokes a private constructor) or returning a boolean and passing your struct to the caller as an out parameter (again through a static method that invokes a private constructor), and so on.

10

It depends a lot. But you should decide to do something and document it.

The only definitively wrong thing for your code to do is to forget to consider that user input might be outside the expected range, and write code that accidentally has some behaviour. Because then some people will make an incorrect assumption about how your code behaves and it will cause bugs, while others will end up depending on the behaviour your code accidentally has (even if that behaviour is completely bonkers) and so you'll cause more bugs when you later fix the problem.

In this case I can see arguments either way. If someone travels +10 degrees from 175 degrees, they should end up at -175. If you always normalise user input and so treat 185 as equivalent to -175 then client code can't do the wrong thing when it adds 10 degrees; it always has the right effect. If you treat 185 as an error you force every case where client code is adding relative degrees to put in the normalisation logic (or at least remember to call your normalisation procedure), you'll actually cause bugs (though hopefully easy to catch ones that will be quickly squashed). But if a longitude number is user-entered, written literally in the program, or calculated through some procedure intended to always be in [-180, 180), then a value outside that range is very likely to indicate an error, so "helpfully" converting it could hide problems.

My ideal in this case would probably be to define a type that represents the correct domain. Use an abstract type (don't let client code simply access the raw numbers inside it), and provide both a normalising and a validating factory (so the client can make the tradeoff). But whatever a value of this type is made, 185 should be indistinguishable from -175 when seen through your public API (doesn't matter whether they're converted on construction or you provide equality, accessors and other operations that ignores the difference somehow).

Ben
  • 1,047
3

If it does not really matter to you to choose one solution, you could just let the user decide.

Given your struct is readonly value object and created by a method/constructor, you could provide two overloads based on the options the user have:

  • Throw an exception (arg out of range)
  • Convert the value to the constraint

Also never let the user have an invalid struct to pass to your other methods, make it right on creation.

Edit: based on the comments, I assume you are using c#.

0

It depends if the input is directly from a user through some UI, or it is from the system.

Input through an UI

It is a user experience question how to handle invalid input. I don't know about your specific case, but in general there are a few options:

  • Alert the user to the error and have the user fix it before proceeding (Most common)
  • Automatically convert to the valid range (if possible), but alert the user to the change and allow the user to verify before proceeding.
  • Silently convert to the valid range and proceed.

The choice depends on the expectations of you users and how critical the data is. For example Google automatically fixes spelling in queries, but this is low risk because a unhelpful change is not a problem and is easy to fix (and even then it is made clear on the result page that the query was changed). On the other hand, if you are entering coordinates for a nuclear missile you might want a more rigid input validation and no silent fixes of invalid data. So there is no universal answer.

Most importantly, you should consider if correcting input even has a benefit for the user. Why would a user enter invalid data? It is easy to see how someone might make a spelling error, but why would anyone enter a longitude of -185? If the user really meant +175 they would probably have typed +175. I think it is most likely that an invalid longitude simply is a typing error, and the user meant -85 or something else. In this case silently converting is bad and unhelpful. The most user friendly approach for you app would probably be to alert the user to the invalid value, and have the user correct it themselves.

Input through an API

If the input is from another system or subsystem, there is no question. You should throw an exception. You should never silent convert invalid input from another system, since it might mask errors elsewhere in the system. If input is "corrected" it should happen in the UI layer, not deeper into the system.

JacquesB
  • 61,955
  • 21
  • 135
  • 189
0

You should throw an exception.

In the example you gave, sending 185 and converting that to -175, then it might be handy in some cases to provide that functionality. But what if the caller sends 1 million? Do they really mean to convert that? It seems more likely that is an error. So if you need to thrown an exception for 1,000,000 but not for 185, then you have to make a decision about an arbitrary threshold for throwing an exception. That threshold is going to trip you up sometime as some calling app is sending values around the threshold.

Better to throw the exception for values out of the range.

brendan
  • 101
-1

Default should be to throw an exception. You could also allow an option like strict=false and do the coercion based on the flag, where of course strict=true is default. This is fairly common:

  • Java DateFormat supports lenient.
  • Gson's JSON parser also supports a lenient mode.
  • Etc.
djechlin
  • 2,212
-1

The most convenient option for a developer would be a compile time error support in the platform, for values out of range. In this case, the range should also be part of method signature, just like the type of the parameters. The same way your API user cannot pass a String if your method signature is defined to take an integer, user should not have been able to pass a value without checking if the value is within the range given in the method signature. If not checked, he should get a compile time error and thus, runtime error could be avoided.

But currently, very few compilers/platforms support this type of compile time checking. So, it's a developers headache. But ideally, your method should just throw a meaningful exception for unsupported values and document it clearly.

BTW, I really love the error model proposed by Joe Duffy here.

Gulshan
  • 9,532
-2

For me best practice is to never change user input. The approach I usually take is to separate validation from execution.

  • Have a simple class that just uses the given parameters.
  • Use a decorator to provide a layer of validation that can be altered at will without affecting the execution class (or else inject a validator if this approach is too difficult).