|
Why not introduce the additional overload when there actually exists at least one implementation of it?
First we'd have this situation:
public abstract class AbstractThing
{
abstract public void Foo(MyOtherThing t);
}
public class ConcreteThing1 : AbstractThing
{
override public void Foo(MyOtherThing t) { ... }
}
Then we'd introduce the additional overload, defaulting to the previously defined behavior when our new exitContext parameter is false:
public abstract class AbstractThing
{
abstract public void Foo(MyOtherThing t);
virtual public void Foo(MyOtherThing t, bool exitContext)
{
if (exitContext)
throw new NotSupportedException();
Foo(t);
}
}
public class ConcreteThing1 : AbstractThing
{
override public void Foo(MyOtherThing t) { ... }
}
At least in this particular case this ought to work fine, and it doesn't break and descendant classes since it doesn't introduce any new abstract members.
|
|
|
|
|
Pay no attention to that code behind the interface.
|
|
|
|
|
While the exitContext bit is indeed a bit icky, I find this line upsetting as well -
if ((millisecondsTimeout < 0) && (millisecondsTimeout != -1))
That should probably be replaced with the functionally equivalent (unless I'm mistaken) and more efficient
if(millisecondsTimeout < -1)
|
|
|
|
|
I don't have a problem with the original version. In fact I think it's better because it shows the intent more clearly i.e. -1 is treated differently to other negative values. 
I'd use the (probably) more efficient version if performance measurements showed it was worthwhile. 
Regards
David R
---------------------------------------------------------------
"Every program eventually becomes rococo, and then rubble." - Alan Perlis
|
|
|
|
|
And I thought the compiler optimization would have caught that one so we'd have the best of both worlds - the more expressive code in our source files, the more efficient code in our assemblies. Apparently it did not, since this source came from disassembly.
|
|
|
|
|
dojohansen wrote: And I thought the compiler optimization would have caught that one
I have no idea what the instruction timings would be like on mega-pipelined processors, but certainly on older machines one could test whether a value was less than zero (i.e. had the high bit set) more quickly than one could test for its being in a particular range. The two-part test could be faster for the case where the value is positive, even if it would be slower for the case where it is negative. If the positive case will occur much more often, eliminating the split test will make the code smaller but could cause it to run more slowly.
|
|
|
|
|
Don't forget that you are looking at reverse engineered code here. The original source code probably didn't say -1 but was:
&& (millisecondsTimeout != Timeout.Infinite)
which avoids hard-coding a magic number into the source code and makes the intent clearer. The compiler will inline this constant value and replace it with -1 so when you reverse engineer it you lose the reference to Timeout.Infinite.
|
|
|
|
|
The compact framework does not support synchronization contexts, and so it is incorrect to set exitContext to true, since the CF cannot do what you are asking it to do in this case.
The ManualResetEvent interface still has the parameter on this method to support migrated code. Any code which passes false to this method will migrate and run on the CF without needing any rework. Any code which passes true is relying on support for synchronization contexts, and so will need to be reworked before it can be migrated to the CF.
|
|
|
|
|
The compact framework does not support synchronization contexts, and so it is incorrect to set exitContext to true, since the CF cannot do what you are asking it to do in this case.
Does the CF not support synchronization contexts at all? My understanding is that the exitContext parameter says to exit any existing synchronization context before waiting, and reacquire it afterward. If no such context can exist, why should a True value of exitContext pose a problem? Code which relies upon synchronization contexts would fail when an attempt was made to create one; if no synchronization context exists, exiting all synchronization contexts should pose no problem.
BTW, with regard to checking for negative values and then for -1, many processors can check for negative numbers more quickly than they can perform general comparisons. The two-part test may save time in the more common case even if it wastes time in the -1 case. Given the likelihood that the -1 was a defined constant, I think that part of the code is reasonable.
|
|
|
|
|
Didn't hold me up for very long, but gave me a chuckle... This is a piece of a messaging system, where the automated clients route incoming messages to more specific handlers, based on what subclass it is...
private void ReceiveMessageAsync(object state)
{
Message m = (Message)state;
if (m is CommandMessage)
OnCommandReceived(m as CommandMessage);
else if (m is ReplyMessage)
OnReplyReceived(m as ReplyMessage);
else if (m is SubscriptionMessage)
{
SubscriptionMessage sm = m as SubscriptionMessage;
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
}
}
I was wondering why commands and replies were going through just fine, but subscription messages would never be routed properly..
Then I remembered what a SubscriptionMessage was...
public class SubscriptionMessage : CommandMessage
Sometimes, order matters 
|
|
|
|
|
Unrelated to the topic at hand, you could just perform the 'as' operation once and save it off in a variable to perform fewer type castings.
private void ReceiveMessageAsync(object state)
{
SubscriptionMessage sm;
CommandMessage cm;
ReplyMessage rm;
if ((sm = (state as SubscriptionMessage)) != null)
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
else if ((cm = (state as CommandMessage)) != null)
OnCommandReceived(cm);
else if ((rm = (state as ReplyMessage)) != null)
OnReplyReceived(rm);
}
|
|
|
|
|
Considered that, but I did some checking around and found that "as" takes a bit more CPU time than "is", so it would be faster for the first branch of the if, and slower for the others.
|
|
|
|
|
Just doing my own comparisons using:
private const int COUNT = 100000000;
public class Base { }
public class Derived : Base { }
static void Main(string[] args)
{
object o = new Derived();
Derived result;
bool b;
Stopwatch sw;
sw = Stopwatch.StartNew();
for (int i = 0; i < COUNT; )
{
if ((result = (o as Derived)) != null)
{
++i;
}
}
sw.Stop();
Console.WriteLine("'As' Elapsed: {0}", sw.ElapsedMilliseconds);
sw = Stopwatch.StartNew();
for (int i = 0; i < COUNT; )
{
if (o is Derived)
++i;
}
sw.Stop();
Console.WriteLine("'Is' Elapsed: {0}", sw.ElapsedMilliseconds);
Console.ReadLine();
}
I get the following results:
'As' Elapsed: 460
'Is' Elapsed: 414
So, basically, it boils down to a 10% savings in time (ignoring the two operations are not on the same footing - 'as' requires an additional command to store the value in result). But because you spend additional time retyping using the 'as', you end up always spending more time. In essence, you might get into the correct branch to know the type, but converting the type blows away all the savings you made. The only case where you end up being faster is when you don't go through any branches and just bail out of the method entirely, so it depends on your usage.
At the very least, you could remove the additional type cast into Message that is not needed.
Ultimately, it is your code. Good read.
|
|
|
|
|
Ok, you inspired me to do some benchmarking of my own...
Didn't think your test was quite accurate, as the declarations would have to be inside the loop, not outside... Didn't make much of a difference, if any.
Then I tried adding two more classes (Derived2, Derived3), declaring a variable of each, testing each in turn, and doing a single "as" in the second part (After it determines the type)... Basically matching the real situation perfectly.
Test object is of type 'Derived':
'As' Elapsed: 453
'Is' Elapsed: 577
Test object is of type 'Derived2' (Second test):
'As' Elapsed: 982
'Is' Elapsed: 1030
Test object is of type 'Derived3':
'As' Elapsed: 1575
'Is' Elapsed: 1529
So there is a SLIGHT gain for using the "is" strategy when testing three or more times, but since the tests would be arranged such that the more common ones are near the top, the numbers to look at would be the middle/average, in this case 'Derived2'... And your method comes out ahead
Gotta send a note to myself to swap it over when I'm back in the office on Monday.
(Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing )
|
|
|
|
|
Ian Shlasko wrote: Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing
I completely disagree, benchmarking and optimising such a case is a complete waste of time, instead look for places where your application could really do with optimisation, work on other features, etc.
While I generally think that Hoare's classic "Premature Optimisation is the Root of All Evil" is too often used to excuse developers from good design (not all optimisation is premature), this may be a classic case where the maxim really applies.
|
|
|
|
|
Hey, I didn't spend much time on it in actual development... But it's an interesting discussion point, in case next time a similar situation pops up, it actually IS important.
I have another routine in a related class that actually handles the routing of messages to/from the different clients and server-side workers... That one sometimes processes 200-300 messages a second (Update notifications and logs, mostly). If that one was doing any sort of casting, something like this might be significant.
So sure, shaving a couple cycles in this particular block of code is insignificant, but it's something to know for the future.
|
|
|
|
|
All calls to reflection are slow in general. In such case, I prefer adding an additional, "constant", enumerated property "type". It is much faster to do a single integer comparison than using the reflection to determine an object's type (which is done in both "is" and "as" operators).
enum MessageTypes { Command, Reply, Subscription }
class Message {
public abstract MessageTypes MessageType { get; };
(...)
}
if (msg.MessageType == MessageTypes.Command) {
}
else if (...)
Greetings - Jacek
|
|
|
|
|
With some more analysis, would it be possible to use virtual functions to take care of this?
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
|
|
|
|
|
Nah, my Message classes are DataContract objects... Don't want anything but basic accessors and mutators in there. And besides, all of those OnXMessage functions ARE virtual... This is the base class for different types of automated clients... The individual jobs just override the handler for the messages they want to react to.
|
|
|
|
|
Some years ago, I worked at the German office of a big international hospital information system manufacturer. The server was written in MUMPS (that's really a disease...), and lots of the code were developed at the US head quarters.
Some day, we had problems in the communication between computers. Eventually we found out that our American colleagues stored IP addresses in two floating point numbers and then concatenated them with a ".", something like address=float1+"."+float2
It worked fine in the US, but in Germany we use a decimal comma instead of a decimal point, and "192,168.109,47" is not a correctly formed IP address...
|
|
|
|
|
Easy to fix. You can use the code we use at our company to convert floats between european/american decimals notation:
Function CAMBIADECIMAL(ByVal NUMERO As String, ByVal TIPO As Integer) As String
Dim I As Integer
Dim J As Integer
Dim ss As String
Dim ss1 As String
Dim PARTE1 As String
Dim PARTE2 As String
Select Case TIPO
Case 1
ss = NUMERO
I = ss.IndexOf(".")
If I > 0 Then
J = ss.Length - I - 1
PARTE1 = ss.Substring(0, I)
PARTE2 = ss.Substring(I + 1, J)
ss1 = PARTE1 + "," + PARTE2
Return (ss1)
End If
Return ss
Case 2
ss = NUMERO
I = ss.IndexOf(",")
If I > 0 Then
J = ss.Length - I - 1
PARTE1 = ss.Substring(0, I)
PARTE2 = ss.Substring(I + 1, J)
ss1 = PARTE1 + "." + PARTE2
Return (ss1)
End If
Return ss
End Select
End Function
|
|
|
|
|
Here in Argentina we also use comma as floating point separator, and the correct code that always works is:
a) to retrieve the current decimal separator:
System.Globalization.CultureInfo.CurrentCulture.NumberFormat.CurrencyDecimalSeparator
b) to output ANY float using "." as its decimal separator
[a float].ToString(System.Globalization.CultureInfo.InvariantCulture.NumberFormat);
c) to parse using an invariant culture, with "." as decimal separator.
System.Convert.ToSingle("3.1415926", System.Globalization.CultureInfo.InvariantCulture.NumberFormat);
|
|
|
|
|
Hey, take it easy! I was just posting another horror 
|
|
|
|
|
Is there any requirement that any particular octet of an IP address always be non-zero? I would think that if e.g. using a subnet mask of 255.255.254.0, an IP address of 192.168.1.0 or 192.168.0.3 would be perfectly valid.
|
|
|
|
|
public class DateTimeExtended
{
protected static readonly Logger Log = NLog.LogManager.GetCurrentClassLogger();
public enum Days { Sun=0, Mon, Tue, Wed, Thu, Fri, Sat };
public static string[] FerialDays = {"0101",
"0501",
"0508",
"0714",
"0815",
"1101",
"1111",
"1225",
};
public static DateTime GetDate()
{
return DateTime.Now;
}
public static string GetDate(string format)
{
try
{
if(String.IsNullOrEmpty(format))
return GetDate().ToString();
else
return GetDate().ToString(format);
}
catch
{
return DateTime.Now.ToString();
}
}
public static string GetHour()
{
return DateTime.Now.Hour.ToString("00") + ":" + DateTime.Now.Minute.ToString("00") + ":" + DateTime.Now.Second.ToString("00");
}
public static int GetDay()
{
return (int) DateTime.Now.Day;
}
public static int GetMonth()
{
return (int) DateTime.Now.Month;
}
public static int GetYear()
{
return (int) DateTime.Now.Year;
}
public static int GetDayOfWeek()
{
return (int) DateTime.Now.DayOfWeek;
}
public static string GetNextMonthStart(DateTime date, int NbMois)
{
DateTime _date = date.AddMonths(NbMois);
return _date.Year.ToString("0000") + "-" + _date.Month.ToString("00") + "-" + "01";
}
public static bool IsDate(DateTime date)
{
try
{
int day = date.Day;
int month = date.Month;
int year = date.Year;
if(month == 2)
{
if(day == 30 || day == 31)
return false;
if(day == 29)
return ((year%4 == 0 && year%100 != 0) || year%400 == 0);
}
if (day == 31)
return !(month == 4 || month == 6 || month == 9 || month == 11);
return true;
}
catch { return false; }
}
public static string GetGuid()
{
return Guid.NewGuid().ToString();
}
public static DateTime GetDateValue(string sVal)
{
DateTime dtRet;
sVal = sVal.Replace("-", "").Replace("/", "");
try
{
int nAdd = 0;
if (Convert.ToInt32(sVal.Substring(0, 2)) < 80)
{
nAdd = 2000;
}
else
{
nAdd = 1980;
}
if (sVal.Length == 6)
{
dtRet = new DateTime(nAdd + Convert.ToInt32(sVal.Substring(0, 2)), Convert.ToInt32(sVal.Substring(2, 2)), Convert.ToInt32(sVal.Substring(4, 2)), 0, 0, 0);
return dtRet;
}
if (sVal.Length == 8)
{
if (sVal.IndexOf("-") > 0)
{
dtRet = new DateTime(nAdd + Convert.ToInt32(sVal.Substring(0, 2)), Convert.ToInt32(sVal.Substring(3, 2)), Convert.ToInt32(sVal.Substring(6, 2)), 0, 0, 0);
return dtRet;
}
dtRet = new DateTime(Convert.ToInt32(sVal.Substring(0, 4)), Convert.ToInt32(sVal.Substring(4, 2)), Convert.ToInt32(sVal.Substring(6, 2)), 0, 0, 0);
return dtRet;
}
if (sVal.Length == 10)
{
dtRet = new DateTime(Convert.ToInt32(sVal.Substring(0, 4)), Convert.ToInt32(sVal.Substring(5, 2)), Convert.ToInt32(sVal.Substring(8, 2)), 0, 0, 0);
return dtRet;
}
return Convert.ToDateTime(sVal);
}
catch (Exception)
{
}
dtRet = new DateTime(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, 0, 0, 0);
return dtRet;
}
}
This code has been in use for years, even if it's totaly uselless.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
