0

I have an app which uses barcode scanning to gather information about fruit.

A barcode could be bound to either an Apple or a Pear: the app asks a remote API which one is it, and then stores the information locally.

Two possibile solutions i have tried (pseudo-code):

1.

enum FruitType {
APPLE,
PEAR

}
class FruitInfo
{
   FruitType Type;
   object Fruit;
}

string barcode = scanBarcode();
FruitInfo fruitInfo = myfruitfetcher.getFruitInfo(barcode);
if (fruitInfo.Type == PEAR){
this.myPear = (Pear) fruitInfo.Fruit;
} else {
this.MyApple = (Apple) fruitInfo.Fruit;
}

The drawback of this solution is implementing a mechanism that allows the casting from the FruitInfo class, which generally means that some kind of custom Json Deserialization has to be implemented.

2.

string barcode = scanBarcode();
FruitType type = fruitTypeFetcher.getFruitType(barcode);
if (type == PEAR){
this.myPear = myPearFetcher.fetchPear(barcode);
} else {
this.MyApple = myAppleFetcher.fetchApple(barcode);
}

The drawback of this solution is that you have to make multiple requests and create multiple endpoints on the server.

Which approach is the best?

3 Answers 3

3

Both your examples suffer from the same OCP violation (link). Specifically:

if (fruitInfo.Type == PEAR){
    //
} else {
    //
}

As your fruit types get extended, this code needs to be altered. That is a problem, because it is a breeding ground for "oh I forgot to adjust X too" kinds of bugs.

The much better approach here is to promote reusability using a common ancestor (base class or interface). Your question doesn't really contain information on what you intend to do with the fruits, so I'm going use a simple example of having the fruit "do a job" (whatever that job is). This logic can of course be changed with what you intend to do with the fruit.

Note that I'm using a base class here, but an interface would be equally fine to use.

public abstract class Fruit
{
    public abstract void DoJob();
}

public class Apple : Fruit
{
    public override void DoJob()
    {
        Console.WriteLine("I'm an apple!");

        // Apple logic
    }
} 

public class Pear : Fruit
{
    public override void DoJob()
    {
        Console.WriteLine("I'm a pear!");
        
        // Pear logic
    }
} 

By separating the apple/pear logic into the apple/pear classes themselves means that you don't violate OCP. Your handling code no longer needs to have branching logic that depends on specific fruit types:

FruitInfo fruitInfo = myfruitfetcher.getFruitInfo(barcode);
Fruit fruit = fruitInfo.Fruit;

this.myFruit = fruit;

// Or, for example:
fruit.DoJob();

Regardless of whether it's an apple or a pear, this code doesn't need to change. Instead, the fruit object itself will figure out what it needs to do when you call DoJob().

Notice how, if we add a third fruit, the above code does not need to change.

public class Banana : Fruit
{
    public override void DoJob()
    {
        Console.WriteLine("I'm a Banana!");

        // Banana logic
    }
} 

That is the core focus of OCP: making code open for extension (= being able to add new variants), while keeping the code closed for modification (= not needing to change existing code).

1
  • Thank you for the detailed answer and explaination
    – Acerbic
    Jul 5, 2021 at 14:04
1

We model for purpose, which is to support the automation we're trying to bring to the domain.  It is this automation that details what level of modeling is needed.

Your examples don't motivate an OOP design with separate classes for pear vs. apple.

For example, if what is being done for automation is tracking inventory (SKU & inventory count), separate classes (for apples vs. pears) are not needed: they can both be modeled as object instances of the same inventory item class.

-1

The one which doesn't cause the computer to do unnecessary work is best.

You are writing a program for a computer to run. Presumably you would like it to run the program efficiently. Don't waste computer time just so you can say you didn't use a cast. Nobody cares whether you used a cast.

But, I wonder why you have two separate fields myPear and MyApple to begin with. Why not myFruit? If your field accepts any Fruit, then you don't have this problem to begin with.

4
  • "Nobody cares whether you used a cast." The developer troubleshooting the reason for an InvalidCastException being thrown will disagree with that statement.
    – Flater
    Jul 5, 2021 at 12:02
  • @Flater Without the cast they'd have to troubleshoot an InvalidAPICallForFruitTypeException
    – user253751
    Jul 5, 2021 at 12:13
  • So a more descriptive exception then. That's still an improvement.
    – Flater
    Jul 5, 2021 at 12:48
  • @Flater well, it could also be a HttpException
    – user253751
    Jul 5, 2021 at 13:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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