New answers tagged

1

Data Corruption Your class is not thread-safe. Using the volatile keyword could cause data corruption because of this. Without this keyword, optimizations might occur that read the field only once in a given scope. Let's have a look at Dequeue. The _headNode is read 3 times into memory, since it's volatile. What if _headNode is not null the 1st read, but ...


4

One of the problems with your approach, is it's not very extensible. If you wanted different rotation factors or even to make a full Caesar Cypher, you would have to totally rewrite your code. A math based approach(Mod 26) can be used to allow any rotation factor, within reasonable limits. Another approach which will allow O(n) space is to convert the ...


8

The number one problem here is that you concatenate translated letters on to your string one at a time. Since strings are immutable in .NET (and most right thinking languages), this means for the first translated character your program allocates space for a new string of length 1 (copying the previous 0-char string), for the second character it allocates ...


3

Without same table and data scripts it is very hard to identify performance issues. However, here are a few observations that might help: Unless I'm reading this wrong, this seems pointless, just adding overhead: ON ((r.ArrivalAirportIATA <> r1.DepartureAirportIATA) OR (r.ArrivalAirportIATA = r1.DepartureAirportIATA)) When using expressions, ...


1

Comparison A comparison of performance of 2 classes only makes sense if both adhere to the same specification. Does your class do what a ConcurrentDictionary does? Review Why would you allow access to the underlying dictionary? If you must allow it, return a IReadOnlyDictionary. Checking arguments before taking a lock prevents unnecessary locks on bad ...


1

Review That level of nesting hurts readability. There is a lack of re-usability for a common set of source urls to redirect. You have specified an ultimate fallback of http://. But what if we encounter https:// or ftp://? Alternative As suggested by others, you want to loop a collection of input values and break on first match. The way I would do it is to ...


2

Review Your code does not clone a collection. It creates one and assigns the values of the specified source collection to it. It also lacks proper generics to make this usable for other types of collections. public T CloneCollection<T>(IEmployees employees) where T : IEmployees, new() { // creates a target collection T collection = new T(); ...


3

private void _timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e) { lock (_syncObject) { if (_isDisposing) { return; } try { _timer.Stop(); Elapsed?.Invoke(sender, e); } finally { _timer.Start(); } } } As ...


1

Review re-organize your code to avoid redundant code segments underlyingNullableType ?? type or DBNull.Value checks I just found out you have a follow-up question: pitty I didn't notice it before :) That one is also review-worthy / can be improved Bugs if (!Enum.IsDefined(.. should not be called. Microsoft Policy: Why is a non defined enum value allowed ...


4

Review property Interval and methods ExternalStart, ExternalStart should throw ObjectDisposedException if _isDisposing is true property Interval and methods ExternalStart, ExternalStart should also acquire a lock on _syncObject when implementing the dispose pattern make sure to include a destructor ~DisposableSafeTimer or seal your class _isDisposing should ...


2

Does the above comply with the Dependency-Inversion Principle? I am .. Yes, because: BLL defines domain classes and interfaces for DAL to implement BLL does not depend on DAL DAL implements BLL-provided interface and uses its domain classes In the Orchestration.cs and FruitService.cs classes I've tried to use Dependency Injection - is my ...


4

Note that some messages from exceptions expand to more lines (e.g. ArgumentNullException), so consider to replace new line characters while formatting the message so each exception message is written neatly in one line: exception.Message.Replace(Environment.NewLine, " ") My version would look something like: public static string GetMessageTree(this ...


3

Review Generic type definition T is redundant. Looping the ExpandoObject is better optimized then looping the destination's properties, because you get the items as key-value pair in a single pass. Fetch only publicly accessible setter properties to avoid nasty exceptions on property.SetValue. Don't be too strict on matching the source and destination's ...


8

Readability first I've tried to condense everything to one line, but am unsure if this is the most efficient way of approaching this problem. Your primary goal when writing code is readability and maintainability and not squeezing everything into single lines. Can you read this? return exception != null ? string.Format("{0}: {1}\n{2}", msgCount, ...


4

Review If the purpose is to output each of the descendant inner exceptions, you are missing all except the first inner exception in case of an AggregateException. And this is a common exception. Don't use \n as new line unless you specifically need to comply to this style of new line. Prefer Environment.NewLine. I am not convinced of the exception != null ->...


3

@dfhwze's answer is great (upvoted it), there are also other options, which refactors a bit more. These could be a option if you need the more than once. Inheritance Create a separate class and add the method there. This is a bit more "information hiding" (OO concept) e.g. public class CountryCodes : Dictionary<string, Dictionary<DateTime, ...


3

The 'problem' with this approach is that you perform a lookup twice for each key. ContainsKey indexer lookup [..] code if (response.ContainsKey("GB") && response["GB"].ContainsKey(new DateTime(2001, 04, 01))) { GB2001 = response["GB"][new DateTime(2001, 04, 01)]; } Use TryGetValue to combine lookup and indexer. string GB2001; if (response....


Top 50 recent answers are included