How could I rework this code so as not to appear as if I am using exceptions as a form of flow-control?

Question

I've heard it said that exceptions and try-catch blocks should not be used to flow-control, so I would like a way to rework this code to avoid that appearance.

I have a method validateTrainingSets, within a neuralNetwork class that does exactly what it's name suggests - validates the prvided training sets (to ensure, among other things, that the number of inputs in the training sets match the number of inputs into the neural network, and that the number of outputs per answer matches the number of final outputs of the neural network).

Because there are many different ways in which the validation can fail, I've devided to leave the throwing of exception up to the validation method itself, and there are three different custom exceptions that can be thrown from the function.

In a method that allows users of the class to manually update the number of neurons per layer, the final bit of code validates and/or updates the training sets (depending on whether or not new training sets were provided), and my question is, how would I improve it?

In the code below, the "true" parameter in the validator tells the validator to set the training sets to null if it fails. (It is intended that if the user did not provide new training sets, but the existing training sets are consistent with the new arrangement to keep them, but to discard them if not).

The fldValidate field is a flag that's set to false earlier in the code and I need to ensure that it's set back to true ragardless of any errors.

try { validateTrainingSets(fldTrainingSets, fldTrainingAnswers, true); }
catch(TrainingSetCardinalityMismatch) { }
catch(InputSetCardinlityMismatch) { }
catch(OutputAnswerCardinalityMismatch) { }
catch {
    fldValidate = true;
    throw;
}
finally { fldValidate = true; }

In general, try/catch/throw should be used for "exceptional" circumstances, and not just for program flow control. The alternative in your case would be to have `validateTrainingSets` return an enumerated type with four choices (Succeeded, TrainingSetCardinalityMismatch, etc). Does that make your code cleaner or not - well, it depends. One thing to realize is that throwing an exception is expensive (writing try/catch/finally is cheap, but the actual throw takes a long time). That may figure into your decision. In any case, this is "opinion-based" and likely to get closed — Flydog57, Jan 23 '19 at 18:17
Readability is King. Other readers will assume an exception is about an error condition. Go for the code that causes the least confusion. — H H, Jan 23 '19 at 18:26
Probably a better question for either Code Review or Software Engineering SE sites. — UnhandledExcepSean, Jan 23 '19 at 18:52
@Flydog57I reworded this question to avoid being opinion based, and to fit some of the suggestions already provided. I've read alot about weather or not exceptions should only be used in "exceptional" cases, & there are people on both sides. The explanation that makes the most sense to me is that if an assumption of a fucntion, for example, is incorrect, rather than return an arbitrary value to indicate that, it's better to throw an exception so others aware of the issue. — Tara, Jan 23 '19 at 21:31
@Flydog57 AlThough, I love the idea of an enum, just need to find a way to also pass along other variables for the exception (like the paramName, mesage, actual value, among other custom fields I created for the exception) , maybe a struct would be more appropriate? — Tara, Jan 23 '19 at 21:32
Definitely codereview.stackexchange.com which is a sister site to stackoverflow is a better place to ask such things as people’s mind set there is less about “right” and “wrong“ and more about “make better”. I asked a question there today and was very happy for the improvements in the answer. — simbo1905, Jan 24 '19 at 00:00

Larry OBrien · Accepted Answer · 2019-01-23T23:49:10.700

Yes, I think most reviewers would find this confusing and ask for changes. If I understand the problem correctly, you might use an enum such as :

enum Validity { Valid, TrainingSetCardinalityMismatch, InputSetCardinalityMismatch, OutputAnswerCardinalityMismatch } 

class TrainingSet { 
    Validity Validate(TrainingAnswer[] trainingAnswers) { // ... etc ... } 
}

And then divvy things up however you find most intuitive, such as :

var validTrainingSets = 
    from trainingSet in fldTrainingSets 
    where trainingSet.Validate(fldTrainingSets) == Validity.Valid
    select trainingSet;

Or :

var validTrainingSets = 
    fldTrainingSelects.Select(t => t.Validate(fldTrainingAnswers)).Filter(v => v == Validity.Valid);

Or :

Dictionary<Validity, List<TrainingSet>> groupedTrainingSets = 
    from trainingSet in fldTrainingSets   
    group trainingSet by trainingSet.Validate(fldTrainingAnswers) into validityGroup 
    select new { validityGroup.Key, validityGroup.ToList() }

If an enum isn't sufficient, then try an interface or an abstract class :

interface IValidity {}

class Valid : IValidity { // ... etc ... } 
class TrainingSetCardinalityMismatch : IValidity { public int ExpectedCardinality; // etc...  }

I love the idea, except I would also need a way to pass other parameters such as the paramName, message, actualValue, etc., among the custom fields that I created for thos particular exceptions, maybe have it return a struct that includes those fields along with a field for the enum? — Tara, Jan 23 '19 at 21:36
I've edited the answer to include the possibility of returning an interface or abstract class. I actually _prefer_ this idea : creating expressive _types_ is a very good practice. There's an even more sophisticated route called the `Either` monad, which would allow you to automatically split the valid and invalid sets, but it's kind of a rabbithole. — Larry OBrien, Jan 23 '19 at 21:44
OH, I see you addressed that with your abstract class, I think this is actually perfect, thanks! — Tara, Jan 23 '19 at 21:45

How could I rework this code so as not to appear as if I am using exceptions as a form of flow-control?

1 Answers1