The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

To throw an exception or not?

Hello,
I'm trying to further hone my use of Exceptions (or non-use, as the case may be). I have a class with a few methods that convert strings of a special format to an int (id). The string has two parts: a type, and an ID. I need to make sure that the string entered is of the type that the page expects. I've got a couple of ways to do this:

Option 1, make the calling class specify the type it expects:
public int getIdFromString(string input, int type)
{
  string type = input.Substring(0,2);

  if(int.Parse(type) != type)
  {
      throw new ArgumentException("String was not of correct type", "input");
  }

  string valPart = input.Substring(2,input.Length-2);
  return int.Parse(valPart)
}

Option 2, make a method so the calling class can check:
public bool isStringOfType(string input, int type)
{
  string type = input.Substring(0,2);
  return int.Parse(type) == type
}

public int getIdFromString(string input)
{
  string valPart = input.Substring(2,input.Length-2);
  return int.Parse(valPart)
}

Option 3, combination:
Use both the getIdFromString(string, int) and isStringOfType(string, int). This gives the calling class a chance to check, but still catches a mistake.

Option 4: ???


Not sure if I made it clear enough, but these are public methods being called by another class. Anyone have any thoughts? I'm leaning towards option 1. Give the calling class the ability to check, and avoid exceptions. The input string comes from user input, so it wouldn't be unheard of if they entered a string of the wrong type.

Thanks for any advice!
Mr. John Shaft
Monday, January 08, 2007
 
 
Without some kind of example, your post is making my head spin.  Can you post a few sample strings that would be valid or invalid?
Kyralessa Send private email
Monday, January 08, 2007
 
 
I'd try to keep knowledge of how ids are turned into strings, and vice-versa, encapsulated in one place, and make all consumers of this logic as oblivious of how it's done as you can.  They don't need to see how the sausage is made.  Therefore, keep the method call simple if you can, and derive type based on input.

Having said that, it sounds like you have multiple ways to convert ID's to strings already, so you should probably make sure this makes sense before you go too far down this road.  Trying to parse meaning out of a string that's not specifically intended to support deserialization can get a bit dicey.

By encapsulating both the encoding and decoding in the same class, and being reasonably sure that no other objects are messing with that representation, your chances are better.
D. Lambert Send private email
Monday, January 08, 2007
 
 
Sorry about that, guess it doesn't make much sense without an example...

string ex1 = "01004";
string ex2 = "01008";
string ex3 = "02010";

ex1: type is 1, id is 4
ex2: type is 1, id is 8
ex3: type is 3, id is 10

The calling class needs to validate that the correct type was used. So, for instance, a certain page expects a string of type 1. So ex1 is good, ex2 is good, ex3 is bad. The question comes down to: when I extract the id, should I verify type, or just give the calling class a way to verify on it's own and assume it's doing its job?

Did that help at all?
Mr. John Shaft
Monday, January 08, 2007
 
 
A good idiom is RAII (resource acquisition is initialization) - you can find a good explanation on wikipedia.

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

Basically - the idea is that if you need to create an object to manage some resources, then all those capabilities should be encapsulated within constructor/destructor, which would provide exception-safe code.

Fewer people follow this idiom in the C# world because of garbage collection, but IMHO you still gain more benefit from this pattern, e.g., making your code tighter and more manageable, without having to explicitly manage the resources.
Yin-So Chen Send private email
Monday, January 08, 2007
 
 
Depends on the context, but usually it's useful to have both - the IsItOK function and the ConvertIt function with the exception.
Mike S Send private email
Monday, January 08, 2007
 
 
Design By Contract would call for the combination method you described. 

The "getIdFromString" method places a responsibility on the caller, so the "isStringOfType" query is necessary to allow the caller to ensure it is meeting its responsibility.  You still want to test this condition in the getIdFromString method, though, to guard against lazy callers who don't take their responsibility seriously, and throw an exception when the contract is violated.  Properly constructed, though, this is an exception you should never see at runtime (only during development).  In fact, most Design by Contract systems would allow you to disable the code for this kind of checking in production if you wanted to.
Mr. Dire Send private email
Monday, January 08, 2007
 
 
I used to work with product configurators, and we ran into something called "smart part numbers" pretty often.  This looks similar.

You're compressing two data elements into one here.  This has to be done with care, since it's really easy to get bitten.  The sources of "bite" generally take two forms:

(1) Logic for encoding and decoding the "smart field" is distributed, and one of the routines is buggy or gets out of synch.  Chaos ensues.

(2) Data conversion.  Something changes -- format of the underlying data elements and/or the routines for assembly and disassembly.  It's difficult to reliably and comprehensively perform a system-wide upgrade due to the embedded nature of this data.  You might be ok here, but with smart part numbers, they had an annoying way of finding their way into systems and documents that weren't relationally linked to the original system in such a way that they could really be converted.  Beware.

It's entirely possible that none of these bad things will happen, of course, but I'd proceed with caution.
D. Lambert Send private email
Monday, January 08, 2007
 
 
If you're using C#, you can follow the TryParse pattern that was added to many classes in version 2.0 of the framework. Essentially looks like:

bool TryParse(string s, out ResultType result)

No exception if it fails, just returns false and the result type is null(or some other default value).
Jason Moore Send private email
Monday, January 08, 2007
 
 
Personally, my first recommendation would be to rethink your data structure.  For the user input, I would provide a drop down list of acceptable types and then use the selected type to drive a class factory to create the appropriate conversion class. 

By having the user select from a list of valid data types, you eliminate the possibility of the user entering a bad type.

If you cannot control the user input, I would still divide your input into a data structure or class with two variables, type and value.  It is just a lot of effort to have one part of the program concatenate the two fields together and then have another part parse the two fields apart.

I don't know how much flexibility you might have to address the user interface, but it is always better to prevent the user from entering a bad value than to catch it later and report an error.
Wayne M.
Tuesday, January 09, 2007
 
 
Hello all, thanks for the responses. I didn't make myself very clear, but they were helpful. Thinking about them made me rethink my way of handling all of this, so I've refactored the code in such a way that it's not a problem. I've made an object that that has two constructors:

(smart string for this example stolen from smart product #s)
(StringTypes is an enum of all possible types)

public SmartString(string stringText)
public SmartString(int id, StringTypes type)

Then if the page needs to see what type the smart string is, I can just expose the StringTypes for that object. The page can then do whatever validation it sees fit, and no exceptions need to be thrown (unless stringText is of the completely wrong format).


Thanks!
Mr. John Shaft
Tuesday, January 09, 2007
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz