4

Looking for maybe some design advice here. I've run into this issue a couple of times now and feel like my approaches have been sub par up until now.

I've got for example a method that I want to create called GetConversation. Now GetConversation will return the same data whether you give it an id or an alternate address parameter. Just a matter of preference or whatever data you have available that makes it a little more flexible and easy to use. This is great but the problem I'm running into is that these are both string parameters. Ideally it would look something like this,

public object GetConversation(string id)
{
    // Get Conversation by id
}

public object GetConversation(string address)
{
    // Get Conversation by address
}

Now of course this is going to fail because of ambiguity. The compiler doesn't know which function to access when the user drills into GetConversation.

This brings me to my second approach at a flexible solution,

public object GetConversation(string id, string address)
{
    // Get Conversation by id or address
    if (id != null)
    {
        // Get by id
    }
    else if (address != null)
    {
        // Get by address
    }

    return null;
}

But now I am starting to make a preference of always searching by id first. Isn't necessarily a bad thing but I feel like this just isn't the most cohesive solution and I have to explicitly set a return outside of my checks for failures on both checks.

Any thoughts on a better approach?

2
  • Suggestion, if by case you have other fields but need Get function like this, why not make like, the first parameter as the type ("id", "address", etc) and the second parameter as the search parameter? Commented Jul 20, 2018 at 5:02
  • 4
    GetConversationById and GetConversationByAddress.
    – mjwills
    Commented Jul 20, 2018 at 5:13

5 Answers 5

6

In this particular scenario, I would rename the methods to be GetConversationById and GetConversationByAddress, to avoid the ambiguity.

If the scenario was slightly different, and the methods were genuine overloads with different implementations, I'd use a strategy pattern with an abstract factory to create the strategies.

1
  • Oh gosh now that you mention this I'm surprised I didn't think of this solution. I see this all over the place. Finally makes sense now.
    – tokyo0709
    Commented Jul 20, 2018 at 7:39
2

An alternative approach you may wish to consider is to introduce new types1. So you would have an Address type and an Id type.

At a minimum, you'd expect these new types to support (explicit) conversions to/from string and implement all of the expected equality and comparison functions2. You then use these types for this function and 1) you're allowed, because now the types are different and 2) It's now clear how the caller makes clear which one they're calling:

var conv = thing.GetConversation((Address)"here");

Now, if that's the only place you'll use these types, it probably doesn't seem worth the effort - but are there more uses for these types? Do Ids or Addresses have more structure beyond merely being strings? (Of course, if it turns out, say, the Addresses must always be valid Uris then it turns out that we didn't need to do all of this work anyway, since somebody already wrote that class for us). If so, we might consider static TryParse methods to allow users to safely test strings.

The more places you can use these types, the fewer times you'll accidentally pass an id when an address was expected, and vice versa.

You also have the opportunity to consider which operations of the underlying type you wish to expose through your wrappers. Sure, concatenating and splitting strings makes sense, but would it make sense to concatenate two ids or two addresses together? (Or, even more bizarre, concatenating an id and an address?). So you only expose the operations that make sense for these new types, and you're steering your consumers into making fewer possible misuses. By providing the explicit operator back to string, there's always a "relief valve" if a consumer does find a need to do something you hadn't thought useful.


1There are some overheads in introducing such wrappers. So I'd not suggest doing this on the hot path for something performance sensitive, but in most situations this would not apply.

2Yes, it does get tedious writing all of the usual boilerplate to implement IEquatable<T> and IComparable<T> but you can make a snippet for this in VS.

1
  • 1
    Late discovery of just one myAddress=yourId bug would cover cost of upfront work of creating separate types. Commented Jul 20, 2018 at 6:32
1

It's clear that you can't overload methods the same signature.

I suggest one of these two approaches if you want compile-time type safety.

(1)

public object GetConversationById(string id)
{
    // Get by id
}

public object GetConversationByAddress(string address)
{
    // Get by address
}

(2)

public object GetConversation(ConversationSource source, string parameter)
{
    switch (source)
    {
        case ConversationSource.Id :
            // Get by id
            break;
        case ConversationSource.Address :
            // Get by address
            break;          
    }
    return null;
}

public enum ConversationSource
{
    Id,
    Address,
}

You'd use these like this:

(1)

var conversation1 = GetConversationById("A123");
var conversation2 = GetConversationByAddress("1 Somewhere St");

(2)

var conversation1 = GetConversation(ConversationSource.Id, "A123");
var conversation2 = GetConversation(ConversationSource.Address, "1 Somewhere St");

To me, option (1) is cleaner.

1
  • For option 2, take a look at using the strategy pattern to implement each case, and pass the enum to an abstract factory to choose the strategy. To use: the code asks the abstract factory to provide the strategy for the enum, and then calls the strategy with the parameter. Commented Jul 20, 2018 at 11:47
0

It depends on your design and how you store your conversation (List, Database, Other Service etc.) and what you are using, but you could define a "generic" method to get your Conversation by using a Predicate (Func<Conversation>) as parameter.

private List<Conversation> _conversations;

public Conversation FindConversation(Func<Conversation> predicate) {
  return _conversations.FirstOrDefault(predicate);
}

So the caller of the method can use it as follows

FindConversation(p=>p.Id == 123);
//or
FindConversation(p=>p.Address == "street");
//or
FindConversation(p=>p.Id = 123 && p.Address == "street");

You then only have one method to get a conversation and do not need to define additional methods if you need to find your conversation by other parameters.

If you store the converations in a database you should use an Expression<Func<Conversation>> as predicate, so that the database provider (IQueryable) could transform the predicate in corresponding query statements (sql, etc.)

0

You can choose Switch Case for the same. It will work for more than two types also.

public object GetConversation(string param, string value)
{
    switch(param)
    {
        case "id":
            // ...
            break;
        case "address":
            // ...
            break;
        default:
            break;
    }

    return null;
}

GetConversation("id", id);
GetConversation("address", address);
2
  • Take a look at the strategy pattern to implement each case, and pass the param to an abstract factory to choose the strategy. Commented Jul 20, 2018 at 5:34
  • @Richardissimo Thanks for suggestion. It was really worth knowing.
    – Karan
    Commented Jul 20, 2018 at 6:12

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.