Kashif
Kashif

Reputation: 14430

Advice on Method overloads

Please see following methods.

    public static ProductsCollection GetDummyData(int? customerId, int? supplierId)
    {
        try
        {
            if (customerId != null && customerId > 0)
            {
                Filter.Add(Customres.CustomerId == customerId);
            }

            if (supplierId != null && supplierId > 0)
            {
                Filter.Add(Suppliers.SupplierId == supplierId);
            }

            ProductsCollection products = new ProductsCollection();

            products.FetchData(Filter);

            return products;
        }
        catch
        {
            throw;
        }
    }

    public static ProductsCollection GetDummyData(int? customerId)
    {
        return ProductsCollection GetDummyData(customerId, (int?)null);
    }

    public static ProductsCollection GetDummyData()
    {
        return ProductsCollection GetDummyData((int?)null);
    }

1- Please advice how can I make overloads for both CustomerId and SupplierId because only one overload can be created with GetDummyData(int? ). Should I add another argument to mention that first argument is CustomerId or SupplierId for example GetDummyData(int?, string). OR should I use enum as 2nd argument and mention that first argument is CustoemId or SupplierId.

2- Is this condition is correct or just checking > 0 is sufficient -> if (customerId != null && customerId > 0)

3- Using Try/catch like this is correct?

4- Passing (int?)null is correct or any other better approach.

Edit:

I have found some other posts like this and because I have no knowledge of Generics that is why I am facing this problem. Am I right? Following is the post.

Overloaded method calling overloaded method

Upvotes: 0

Views: 137

Answers (2)

LukeH
LukeH

Reputation: 269358

  1. Why not just create separate GetCustomerData(int) and GetSupplierData(int) methods? (Along with GetData() and GetData(int,int) if you need them.)

  2. If you change your method arguments to int rather than int? then you only need to check (at your discretion) that they're greater than 0.

  3. There's no need for the try...catch in this situation. If all you're doing is re-throwing the exception then don't bother catching it in the first place.

  4. See 1 and 2 above.

EDIT: Maybe something like this...

public static ProductsCollection GetData()
{
    return GetDataImpl(-1, -1);
}

public static ProductsCollection GetData(int customerId, int supplierId)
{
    if (customerId <= 0)
        throw new ArgumentOutOfRangeException("customerId");

    if (supplierId <= 0)
        throw new ArgumentOutOfRangeException("supplierId");

    return GetDataImpl(customerId, supplierId);
}

public static ProductsCollection GetCustomerData(int customerId)
{
    if (customerId <= 0)
        throw new ArgumentOutOfRangeException("customerId");

    return GetDataImpl(customerId, -1);
}

public static ProductsCollection GetSupplierData(int supplierId)
{
    if (supplierId <= 0)
        throw new ArgumentOutOfRangeException("supplierId");

    return GetDataImpl(-1, supplierId);
}

private static ProductsCollection GetDataImpl(int customerId, int supplierId)
{
    if (customerId > 0)
        Filter.Add(Customers.CustomerId == customerId);

    if (supplierId > 0)
        Filter.Add(Suppliers.SupplierId == supplierId);

    ProductsCollection products = new ProductsCollection();
    products.FetchData(Filter);

    return products;
}

Upvotes: 3

kemiller2002
kemiller2002

Reputation: 115488

You aren't doing anything with the try catch, so why include it. You exception is going to bubble up to the calling method anyway, so just remove the extraneous try catch code.

I would ditch the second and the third methods as they aren't really doing anything. Unless you have some reason to keep them around, like they are legacy code, I would remove them. It will just make the code more complicated for future support.

Upvotes: 1

Related Questions