rhianwen
asked on
Best practice - to overload or not to overload?
I have the following 3 funuctions which as you can see do almost (but not quite) the same things, and accept different paramaters. At the moment, they're also named differently. And to be honest, it seems kind of ugly. However, I'm not sure what the best practice way to achieve this would be. Any suggestions?
[WebMethod(), SoapHeader("sHeader")]
public void SendMessages(Int64 userID, Int32 serviceID, String[] messageInfo)
{
if (sHeader.AuthenticateUser())
{
Subscriptions theSubscription = new Subscriptions();
theSubscription.UserID = userID;
theSubscription.ServiceID = serviceID;
theSubscription.GetSubscriptionDetails();
if (theSubscription.IsSubscribed())
{
Messages theMessage = new Messages();
theMessage.MessageRecipient = theSubscription.MobileNumber;
theMessage.ServiceID = theSubscription.ServiceID;
theMessage.SubscriptionID = theSubscription.SubscriptionID;
theMessage.MessageParameters = messageInfo;
theMessage.SendMessage("serviceMsg", true, true);
}
else
{
//this user is not subscribed you can't message them...
theSubscription.NotifyAxionUnsubscribe();
}
}
else
{
//unauthorized consumption of webservice!
ErrorHandler.HandleError(76, HttpContext.Current.Request.UserHostAddress, this, "");
}
}
[WebMethod(), SoapHeader("sHeader")]
public void SendThankyouMessages(Int64 userID, Int32 serviceID)
{
if (sHeader.AuthenticateUser())
{
Subscriptions theSubscription = new Subscriptions();
theSubscription.UserID = userID;
theSubscription.ServiceID = serviceID;
theSubscription.GetSubscriptionDetails();
if (theSubscription.IsSubscribed())
{
Messages theMessage = new Messages();
theMessage.MessageRecipient = theSubscription.MobileNumber;
theMessage.ServiceID = theSubscription.ServiceID;
theMessage.SubscriptionID = theSubscription.SubscriptionID;
theMessage.SendMessage("thankyou", true, false);
}
else
{
//this user is not subscribed you can't message them...
theSubscription.NotifyAxionUnsubscribe();
}
}
else
{
//unauthorized consumption of webservice!
ErrorHandler.HandleError(76, HttpContext.Current.Request.UserHostAddress, this, "");
}
}
[WebMethod(), SoapHeader("sHeader")]
public void SendStandardMessages(Int64 userID, Int32 serviceID, String message)
{
if (sHeader.AuthenticateUser())
{
Subscriptions theSubscription = new Subscriptions();
theSubscription.UserID = userID;
theSubscription.ServiceID = serviceID;
theSubscription.GetSubscriptionDetails();
if (theSubscription.IsSubscribed())
{
Messages theMessage = new Messages();
theMessage.MessageRecipient = theSubscription.MobileNumber;
theMessage.ServiceID = theSubscription.ServiceID;
theMessage.SubscriptionID = theSubscription.SubscriptionID;
theMessage.MessageBody = message;
theMessage.SendMessage("standard", false, false);
}
else
{
//this user is not subscribed you can't message them...
theSubscription.NotifyAxionUnsubscribe();
}
}
else
{
//unauthorized consumption of webservice!
ErrorHandler.HandleError(76, HttpContext.Current.Request.UserHostAddress, this, "");
}
}
Last Comment
ASKER
The only problem with that is that the function is part of a webservice, and the site calling the service does not know anything about the message object. I'm unsure how I would pass in the object given this is the case.
Is it possible to do something like
public void SendMessages(Int64 userID, Int32 serviceID, String[] messageInfo)
{
...
}
Calls to other two functions can be replaced as
SendStandardMessages(Int64
SendThankyouMessages(Int64
public void SendMessages(Int64 userID, Int32 serviceID, String[] messageInfo)
{
...
}
Calls to other two functions can be replaced as
SendStandardMessages(Int64
SendThankyouMessages(Int64
ASKER
I considered that, but then I'm not sure how to create a method that I can call from each of those functions that would do all the subscription/message stuff - I would still be replicating most of the code in each of the functions
ASKER
The problem is that for the creation of the message content I'm using either a String, a String[] or nothing... so a function to replace the content of all 3 SendMessage functions would need to accept all those types - but I don't think its very good to create a function that has all those parameters and then pass empty variables to them depending on the type of message?
for example
private void doMessage (String type, String[] message, String parameters)
{
...
}
then from SendMessages I would do doMessage("serviceMsg", messageInfo, "");
and from SendThankyouMessage doMessage("thankyou", new String[], "");
and StandardMessage doMessage("standard", new String[], message);
for example
private void doMessage (String type, String[] message, String parameters)
{
...
}
then from SendMessages I would do doMessage("serviceMsg", messageInfo, "");
and from SendThankyouMessage doMessage("thankyou", new String[], "");
and StandardMessage doMessage("standard", new String[], message);
ASKER
Ok I think I might have found a solution
If I pass an object to the doMessage function, I can use the type of the object to determine which actions to take
eg
private void doMessage (Object objMsg)
{
if (typeof(objMsg) is String)
{}
else if (typeof(objMsg) is String[])
{}
}
what do you think?
If I pass an object to the doMessage function, I can use the type of the object to determine which actions to take
eg
private void doMessage (Object objMsg)
{
if (typeof(objMsg) is String)
{}
else if (typeof(objMsg) is String[])
{}
}
what do you think?
You dont even need those checks ... have a superclass message that has an abstract function getMessageString. Each message class derives from this superclass and overrides this abstract function. Your sendMessage function can simply call this function to get the string to send.
ASKER
To be honest, I don't really understand that LOL but I'm pretty sure its overkill for my situation.
I've gone from what was above to the code shown below - maybe not technically perfect but I think its a big improvement
I've gone from what was above to the code shown below - maybe not technically perfect but I think its a big improvement
[WebMethod(), SoapHeader("sHeader")]
public void SendMessage(Int64 userID, Int32 serviceID, String[] messageInfo)
{
DoSend(userID, serviceID, messageInfo, true, true);
}
private void DoSend(Int64 userID, Int32 serviceID, Object objMsg, Boolean premium, Boolean includeInTotal)
{
if (sHeader.AuthenticateUser())
{
Subscriptions theSubscription = new Subscriptions();
theSubscription.UserID = userID;
theSubscription.ServiceID = serviceID;
theSubscription.GetSubscriptionDetails();
if (theSubscription.IsSubscribed())
{
Messages theMessage = new Messages();
theMessage.MessageRecipient = theSubscription.MobileNumber;
theMessage.ServiceID = theSubscription.ServiceID;
theMessage.SubscriptionID = theSubscription.SubscriptionID;
switch (objMsg.GetType().Name)
{
//using message paramaters so its a serviceMsg
case "String[]":
theMessage.MessageParameters = (String[])objMsg;
theMessage.SendMessage("serviceMsg", true, true);
break;
//string to its a standard message
case "String":
theMessage.MessageBody = (String)objMsg;
theMessage.SendMessage("standard", false, false);
break;
// new object so its a thankyou message
case "Object":
theMessage.SendMessage("thankyou", true, false);
break;
}
}
else
{
//this user is not subscribed you can't message them...
theSubscription.NotifyAxionUnsubscribe();
}
}
else
{
//unauthorized consumption of webservice!
ErrorHandler.HandleError(76, HttpContext.Current.Request.UserHostAddress, this, "");
}
}
[WebMethod(), SoapHeader("sHeader")]
public void SendMessage(Int64 userID, Int32 serviceID)
{
DoSend(userID, serviceID, new Object(), true, false);
}
[WebMethod(), SoapHeader("sHeader")]
public void SendMessage(Int64 userID, Int32 serviceID, String message)
{
DoSend(userID, serviceID, message, false, false);
}
ASKER
Damn I knew there was a reason I hadn't overloaded the methods until now. In a webservice overloading methods is not supported - at least not unless you have a unique messagename for each, and that seems to be broken for me as I get a "does not conform to WS-I basic profile v1.1" error.
When building a webservice that is not inherent to the client application, it seems to make plenty of sense to make the user interface as clear as possible.
As a user of your webservice, "SendMessages", "SendThankyouMessages", "SendStandardMessages" are so very obvious and helpful. If behind the scenes (your non-public interface) you want to overload some routine, then so be it. But as a public user of your interface, I would personally appreciate the clarity of separate routines.
-Cheers,
Joe
As a user of your webservice, "SendMessages", "SendThankyouMessages", "SendStandardMessages" are so very obvious and helpful. If behind the scenes (your non-public interface) you want to overload some routine, then so be it. But as a public user of your interface, I would personally appreciate the clarity of separate routines.
-Cheers,
Joe
While I do not claim any expertise with webservices, but as a programmer, I find and interface like sendmessage ("message"); lot more intuitive than having to remember a bunch of send functions which differ only in the message that needs to be sent.
If function overloading breaks your web service compliance, you can use a single API http:#22056987
If function overloading breaks your web service compliance, you can use a single API http:#22056987
ASKER CERTIFIED SOLUTION
THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
SOLUTION
THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
Yep, exactly ... and this interface is lot cleaner than having multiple APIs which are doing essentially the same thing but on different parameters.
Ummm .. okay I see a point .. ideally I would have liked an overloaded message factory, but that again is not possible in this case.
SOLUTION
THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
ASKER
Wow, some great answers thanks everyone! Its interesting to see that everyone has different ideas on what is the "best" way to do this!
C#
C# is an object-oriented programming language created in conjunction with Microsoft’s .NET framework. Compilation is usually done into the Microsoft Intermediate Language (MSIL), which is then JIT-compiled to native code (and cached) during execution in the Common Language Runtime (CLR).
98K
Questions
--
Followers
--
Top Experts
Get a personalized solution from industry experts
TRUSTED BY
All three methods send a message. Overloading would definitely make it better. However, it may not be best to have different send function based on type of message. An alternative is to have a single send function that takes serviceId, usedID and a message object. The message object itself can be constructed from a string or any other source you like.
Regards,
sunnycoder