locked
2 Methods Same Signature But do 2 different things RRS feed

  • Question

  • Initially I had a method in our DL that would take in the object it's updating like so:

        internal void UpdateCash(Cash Cash)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    captureID = @captureID,
                                            ac_code = @acCode,
                                            captureDate = @captureDate,
                                            errmsg = @errorMessage,
                                            isDebit = @isDebit,
                                            SourceInfoID = @sourceInfoID,
                                            PayPalTransactionInfoID = @payPalTransactionInfoID,
                                            CreditCardTransactionInfoID = @CreditCardTransactionInfoID
                                         where id = @cashID";

                conn.AddParam("@captureID", cash.CaptureID);
                conn.AddParam("@acCode", cash.ActionCode);
                conn.AddParam("@captureDate", cash.CaptureDate);
                conn.AddParam("@errorMessage", cash.ErrorMessage);
                conn.AddParam("@isDebit", cyberCash.IsDebit);
                conn.AddParam("@PayPalTransactionInfoID", cash.PayPalTransactionInfoID);
                conn.AddParam("@CreditCardTransactionInfoID", cash.CreditCardTransactionInfoID);
                conn.AddParam("@sourceInfoID", cash.SourceInfoID);
                conn.AddParam("@cashID", cash.Id);

                conn.ExecuteNonQuery();
            }
        }

    My boss felt that creating an object every time just to update one or two fields is overkill.  But I had a couple places in code using this.  He recommended using just UpdateCash and sending in the ID for CAsh and field I want to update.  Well the problem is I have 2 places in code using my original method.  And those 2 places are updating 2 completely different fields in the Cash table.  Before I was just able to get the existing Cash record and shove it into a Cash object, then update the properties I wanted to be updated in the DB, then send back the cash object to my method above.

    I need some advice on what to do here.  I have 2 methods and they have the same signature.  I'm not quite sure what to rename these because both are updating 2 completely different fields in the Cash table:

        internal void UpdateCash(int cashID, int paypalCaptureID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    CaptureID = @paypalCaptureID
                          where id = @cashID";
        
                conn.AddParam("@captureID", paypalCaptureID);
        
                conn.ExecuteNonQuery();
            }
        }

        internal void UpdateCash(int cashID, int PayPalTransactionInfoID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    PaymentSourceID = @PayPalTransactionInfoID
                          where id = @cashID";
        
                conn.AddParam("@PayPalTransactionInfoID", PayPalTransactionInfoID);
        
                conn.ExecuteNonQuery();
            }
        }

    So I thought hmm, maybe change the names to these so that they are now unique and somewhat explain what field its updating:

        UpdateCashOrderID
        
        UpdateCashTransactionInfoID

    ok but that's not really very good names.  And I can't go too generic, for example:

        UpdateCashTransaction(int cashID, paypalTransactionID)

    What if we have different types of transactionIDs that the cash record holds besides just the paypalTransactionInfoID?  such as the creditCardInfoID?  Then what?  Transaction doesn't tell me what kind.  And furthermore what if you're updating 2 fields so you have 2 params next to the cashID param:

        UpdateCashTransaction(int cashID, paypalTransactionID, someOtherFieldIWantToUpdate)

    see my frustration?  what's the best way to handle this is my boss doesn't like my first route?

    • Edited by NoEgo Wednesday, March 31, 2010 8:46 PM
    Wednesday, March 31, 2010 7:47 PM

Answers

  • Let's that enum from up top there, TransactionType.

        interface ITransaction
        {
            void PayCash(int cashID, int infoID);
        }
        abstract class ATransaction : ITransaction
        {
            public abstract void PayCash(int cashID, int infoID);
        }
        class CaptureTransaction : ATransaction
        {
            public override void PayCash(int cashID, int paypalCaptureID)
            {
                throw new NotImplementedException();
            }
        }
        class InfoTransaction : ATransaction
        {
            public override void PayCash(int cashID, int PayPalTransactionInfoID)
            {
                throw new NotImplementedException();
            }
        }
    
        class Consumer
        {
            Dictionary<TransactionType, ITransaction> transactions;
            public Consumer()
            {
                transactions = new Dictionary<TransactionType, ITransaction>();
                transactions.Add(TransactionType.CaptureId, new CaptureTransaction());
                transactions.Add(TransactionType.TransactionId, new InfoTransaction());
            }
            public void PayCash(int cashID, int infoID, TransactionType transactionType)
            {
                transactions[transactionType].PayCash(cashID, infoID);
            }
        }

    There are more abstract ways to fill the collection.  You could use a collection of delegates pointing at static methods.  You could use Reflection to fill the collection.  Etc.


    Mark the best replies as answers. "Fooling computers since 1971."
    • Proposed as answer by Ji.Zhou Tuesday, April 6, 2010 1:58 PM
    • Marked as answer by Ji.Zhou Thursday, April 8, 2010 3:28 AM
    Thursday, April 1, 2010 1:58 PM
  • That's halfway to a fully configurable provider model, good thinking. Switch out that dictionary for a "TransactionProviders" configuration section and one or more providers in a single call and remove the transaction type enum all together.

    Perhaps something like this...

    <TransactionProviders>
    	<add name="PayPal"
    		type="MyCo.Financial.Transation.Providers.PayPalProvider, Financial.Transation.Providers" />
    	<add name="GoogleCheckout"
    		type="MyCo.Financial.Transation.Providers.GoogleCheckoutProvider, Financial.Transation.Providers" />
    </TransactionProviders>

    Then change the consumer...

    public class Consumer
    {
    	private List<ITransaction> transactionProviders = null;
    	
    	public Consumer()
    	{
    		transactionProviders = new List<ITransaction>();
    		// TODO: Load transaction providers from config
    	}
    	
    	public void PayCash(int cashID, int infoID)
    	{
    		foreach(ITransaction provider in transactionProviders)
    			provider.PayCash(cashID, infoID);
    	}
    }
    Now there's an unlimited number of providers supported with the ability to add new ones into the mix of makes changes without recompiling and redistributing the entire application.
    "There's a way to do it better - find it." - Thomas Edison
    • Marked as answer by Rudedog2 Friday, April 9, 2010 12:59 PM
    Thursday, April 1, 2010 2:28 PM

All replies

  • I personally would rename them both.

     

    I would rename one as UpdateCashFromOrder

    I and I would name the other UpdateCashFromTransaction.

     

    The key is to make the name have meaning.

    Wednesday, March 31, 2010 8:02 PM
  • You can support even more columns by...
            internal void UpdateCash(int cashID, string columnName, object value)
            {
                using (OurCustomDBConnection conn = CreateConnection("UpdateCash"))
                {
                    conn.CommandText = string.Format("update Cash set [{0}] = @{1} where id = @cashID", columnName, columnName.Replace(" ", "_"));
    
                    conn.AddParam("@cashID", cashID);
                    conn.AddParam(string.Format("@{0}", columnName.Replace(" ", "_")), value);
    
                    conn.ExecuteNonQuery();
                }
            }
    

    My Blog - MSDN Complement By Providing Visual C# Walkthroughs and Sample Codes - Founded In February 24, 2010
    Wednesday, March 31, 2010 8:07 PM
  • Yasser, interesting but then you'd have possibly a bunch of magic strings all throughout your code for sending in the columnName
    C# Web Developer
    Wednesday, March 31, 2010 8:13 PM
  • I responded to Derik a few minutes ago on the side.  What if you have more than one type of transaction in your business process?  For instance a home grown order transaction vs. a PayPal transaction.  Then Transaction doesn't tell you enough to what type it's handling for you.

    so then what???

    Do you see my dilemma here?


    C# Web Developer
    Wednesday, March 31, 2010 8:14 PM
  • All, I just updated the original post.  Please re-read now, it has more context and background behind all this.
    C# Web Developer
    Wednesday, March 31, 2010 8:46 PM
  • This sounds a bit like a naming issue, a search for graceful and meaningful code...I love it. How does this strike you?

    /// <summary>Enumeration of supported transaction types.</summary>
    public enum TransactionType
    {
        CaptureId = 1,
        TransactionId = 2
    }
    
    /// <summary>Enumeration of supported financial systems.</summary>
    public enum SystemType
    {
        PayPal = 1,
        Internal = 2
    }
    
    /// <summary>Updates the specified cash identifier.</summary>
    /// <param name="cashID">The cash record identifier.</param>
    /// <param name="infoId">The transaction record identifier.</param>
    /// <param name="system">The system type.</param>
    /// <param name="type">The transaction type.</param>
    public void UpdateCash(int cashID, int infoId, SystemType system, TransactionType type)
    {
        // TODO: Call internal update methods named anything you want.
    }

    "There's a way to do it better - find it." - Thomas Edison
    • Edited by Gio Palacino Wednesday, March 31, 2010 9:03 PM Typo
    Wednesday, March 31, 2010 9:03 PM
  • you add commantaries

        /// <summary>
        ///
        /// </summary>
        /// <param name="cashID"></param>
        /// <param name="paypalCaptureID"></param>
        internal void UpdateCash(int cashID, int paypalCaptureID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    CaptureID = @paypalCaptureID
                          where id = @cashID";

                conn.AddParam("@captureID", paypalCaptureID);

                conn.ExecuteNonQuery();
            }
        }

        /// <summary>
        ///
        /// </summary>
        /// <param name="cashID"></param>
        /// <param name="PayPalTransactionInfoID"></param>
        internal void UpdateCash(int cashID, int PayPalTransactionInfoID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    PaymentSourceID = @PayPalTransactionInfoID
                          where id = @cashID";

                conn.AddParam("@PayPalTransactionInfoID", PayPalTransactionInfoID);

                conn.ExecuteNonQuery();
            }
        }

    Thursday, April 1, 2010 11:23 AM
  • For example

        /// <summary>
        /// Update PaypalCaptureID Value for Cash Table
        /// </summary>
        /// <param name="cashID"></param>
        /// <param name="paypalCaptureID"></param>
        internal void UpdateCash(int cashID, int paypalCaptureID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    CaptureID = @paypalCaptureID
                          where id = @cashID";

                conn.AddParam("@captureID", paypalCaptureID);

                conn.ExecuteNonQuery();
            }
        }

     

        /// <summary>
        /// Update PayPalTransactionInfoID of Cash Table
        /// </summary>
        /// <param name="cashID"></param>
        /// <param name="PayPalTransactionInfoID"></param>
        internal void UpdateCash(int cashID, int PayPalTransactionInfoID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    PaymentSourceID = @PayPalTransactionInfoID
                          where id = @cashID";

                conn.AddParam("@PayPalTransactionInfoID", PayPalTransactionInfoID);

                conn.ExecuteNonQuery();
            }
        }

    -----------------------------------------------------------------------------------------

    or you can add an other parameter like:     internal void UpdateCash(int cashID, int newColumnValue, string columnName)

    and do a switch case

    Thursday, April 1, 2010 11:33 AM
  • Unfortunately, adding comments won't help in this case. You see when the methods "UpdateCash(int cashID, int PayPalTransactionInfoID)" and "UpdateCash(int cashID, int paypalCaptureID)" are compiled into IL the signatures will be "UpdateCash(int, int)" there are no parameter names. Simply having the same method name with the same number of parameters in the same sequence of the same type makes the method signatures non-unique.

    Either better names are needed or different signatures, however, comments are ALWAYS a good idea in my opinion.


    "There's a way to do it better - find it." - Thomas Edison
    • Edited by Gio Palacino Thursday, April 1, 2010 1:16 PM Typo
    Thursday, April 1, 2010 1:16 PM
  • Initially I had a method in our DL that would take in the object it's updating like so:

        internal void UpdateCash(Cash Cash)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    captureID = @captureID,
                                            ac_code = @acCode,
                                            captureDate = @captureDate,
                                            errmsg = @errorMessage,
                                            isDebit = @isDebit,
                                            SourceInfoID = @sourceInfoID,
                                            PayPalTransactionInfoID = @payPalTransactionInfoID,
                                            CreditCardTransactionInfoID = @CreditCardTransactionInfoID
                                         where id = @cashID";

                conn.AddParam("@captureID", cash.CaptureID);
                conn.AddParam("@acCode", cash.ActionCode);
                conn.AddParam("@captureDate", cash.CaptureDate);
                conn.AddParam("@errorMessage", cash.ErrorMessage);
                conn.AddParam("@isDebit", cyberCash.IsDebit);
                conn.AddParam("@PayPalTransactionInfoID", cash.PayPalTransactionInfoID);
                conn.AddParam("@CreditCardTransactionInfoID", cash.CreditCardTransactionInfoID);
                conn.AddParam("@sourceInfoID", cash.SourceInfoID);
                conn.AddParam("@cashID", cash.Id);

                conn.ExecuteNonQuery();
            }
        }

    My boss felt that creating an object every time just to update one or two fields is overkill.  But I had a couple places in code using this.  He recommended using just UpdateCash and sending in the ID for CAsh and field I want to update.  Well the problem is I have 2 places in code using my original method.  And those 2 places are updating 2 completely different fields in the Cash table.  Before I was just able to get the existing Cash record and shove it into a Cash object, then update the properties I wanted to be updated in the DB, then send back the cash object to my method above.

    I need some advice on what to do here.  I have 2 methods and they have the same signature.  I'm not quite sure what to rename these because both are updating 2 completely different fields in the Cash table:

        internal void UpdateCash(int cashID, int paypalCaptureID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    CaptureID = @paypalCaptureID
                          where id = @cashID";
        
                conn.AddParam("@captureID", paypalCaptureID);
        
                conn.ExecuteNonQuery();
            }
        }

        internal void UpdateCash(int cashID, int PayPalTransactionInfoID)
        {
            using (OurCustomDbConnection conn = CreateConnection("UpdateCash"))
            {
                conn.CommandText = @"update Cash
                                     set    PaymentSourceID = @PayPalTransactionInfoID
                          where id = @cashID";
        
                conn.AddParam("@PayPalTransactionInfoID", PayPalTransactionInfoID);
        
                conn.ExecuteNonQuery();
            }
        }

    So I thought hmm, maybe change the names to these so that they are now unique and somewhat explain what field its updating:

        UpdateCashOrderID
        
        UpdateCashTransactionInfoID

    ok but that's not really very good names.  And I can't go too generic, for example:

        UpdateCashTransaction(int cashID, paypalTransactionID)

    What if we have different types of transactionIDs that the cash record holds besides just the paypalTransactionInfoID?  such as the creditCardInfoID?  Then what?  Transaction doesn't tell me what kind.  And furthermore what if you're updating 2 fields so you have 2 params next to the cashID param:

        UpdateCashTransaction(int cashID, paypalTransactionID, someOtherFieldIWantToUpdate)

    see my frustration?  what's the best way to handle this is my boss doesn't like my first route?


    Maybe the methods need to be in different classes.  Maybe the method needs to be defined by an interface.

    Mark the best replies as answers. "Fooling computers since 1971."
    Thursday, April 1, 2010 1:28 PM
  • Let's that enum from up top there, TransactionType.

        interface ITransaction
        {
            void PayCash(int cashID, int infoID);
        }
        abstract class ATransaction : ITransaction
        {
            public abstract void PayCash(int cashID, int infoID);
        }
        class CaptureTransaction : ATransaction
        {
            public override void PayCash(int cashID, int paypalCaptureID)
            {
                throw new NotImplementedException();
            }
        }
        class InfoTransaction : ATransaction
        {
            public override void PayCash(int cashID, int PayPalTransactionInfoID)
            {
                throw new NotImplementedException();
            }
        }
    
        class Consumer
        {
            Dictionary<TransactionType, ITransaction> transactions;
            public Consumer()
            {
                transactions = new Dictionary<TransactionType, ITransaction>();
                transactions.Add(TransactionType.CaptureId, new CaptureTransaction());
                transactions.Add(TransactionType.TransactionId, new InfoTransaction());
            }
            public void PayCash(int cashID, int infoID, TransactionType transactionType)
            {
                transactions[transactionType].PayCash(cashID, infoID);
            }
        }

    There are more abstract ways to fill the collection.  You could use a collection of delegates pointing at static methods.  You could use Reflection to fill the collection.  Etc.


    Mark the best replies as answers. "Fooling computers since 1971."
    • Proposed as answer by Ji.Zhou Tuesday, April 6, 2010 1:58 PM
    • Marked as answer by Ji.Zhou Thursday, April 8, 2010 3:28 AM
    Thursday, April 1, 2010 1:58 PM
  • That's halfway to a fully configurable provider model, good thinking. Switch out that dictionary for a "TransactionProviders" configuration section and one or more providers in a single call and remove the transaction type enum all together.

    Perhaps something like this...

    <TransactionProviders>
    	<add name="PayPal"
    		type="MyCo.Financial.Transation.Providers.PayPalProvider, Financial.Transation.Providers" />
    	<add name="GoogleCheckout"
    		type="MyCo.Financial.Transation.Providers.GoogleCheckoutProvider, Financial.Transation.Providers" />
    </TransactionProviders>

    Then change the consumer...

    public class Consumer
    {
    	private List<ITransaction> transactionProviders = null;
    	
    	public Consumer()
    	{
    		transactionProviders = new List<ITransaction>();
    		// TODO: Load transaction providers from config
    	}
    	
    	public void PayCash(int cashID, int infoID)
    	{
    		foreach(ITransaction provider in transactionProviders)
    			provider.PayCash(cashID, infoID);
    	}
    }
    Now there's an unlimited number of providers supported with the ability to add new ones into the mix of makes changes without recompiling and redistributing the entire application.
    "There's a way to do it better - find it." - Thomas Edison
    • Marked as answer by Rudedog2 Friday, April 9, 2010 12:59 PM
    Thursday, April 1, 2010 2:28 PM