38

is the current implementation 'safe'? Absolutely not. The fact that you had to ask this question indicates that you do not understand enough about threading to build your own mechanisms like this. You need to have a deep and thorough understanding of the memory model to build these mechanisms. That is why you should always rely on the mechanisms provided ...


18

Having backported Lazy to .NET 2.0, I want to offer a variant... First of all, this looks a lot like LazyInitializer. If that works for you, then there is no need to go about creating a Lazy<T> variant. With that said, let's continue... Before going into the code, let us see some things that may go wrong: The ABA problem Thread visibility/Code ...


13

is there anything else I'm missing that I should be concerned about in a high traffic application? Yes, your Lazy<T>.Value isn't generic anymore but an object and if Func<T> returns a value type then a lot of un/boxing will take place. This might hurt performance. I think a LazyFactory.GetOrCreate<T>(...) would do a better job.


11

There's certainly room for improvement. Move that that Select(stringFieldFunction).ToList() out of the while loop. Iterating an IEnumerable might be quite expensive, and there's no need to repeat that work when you find a duplicate. Select only those strings that have originalString as a prefix. That will reduce the amount of items you need to check against ...


10

Meaning that I have to put half of my logic concerning the lazy property in the constructor, and having more boilerplate code. This is a little speculative, but I think you have an XY problem. You're trying to reduce boilerplate, but there are probably better ways to do that than what you've suggested. If I understand correctly, your problem is that your ...


9

I like the idea, but you should carefully explain how this works in comments. Try this: MyLazy myLazy = new MyLazy(); int value1 = myLazy.Get(() => 42); Console.WriteLine(value1); int value2 = myLazy.Get(() => 65); Console.WriteLine(value2); It correctly prints out: 42 42 But even that we know the answer to everything is 42, it isn't ...


8

- float float constants need the suffix f: static const float EXPANSION_POINT = 1.0f; if not, you're assigning a double constant implicitly converted to float. - functions that accept 0 parameters Functions that accept 0 parameters should always be defined as type foo(void) type foo() means different things depending on the context. It is different ...


7

You said you built this specifically so you can use dictionaries as keys in other dictionaries? In that case, you've got a problem. Consider the following demonstration: var keyA = new Dictionary<string, int> { ["a"] = 4 }; var keyB = new Dictionary<string, int> { ["a"] = 4 }; var comparer = new DictionaryEqualityComparer<string, int>(); ...


7

I wouldn't say, your major concern should be performance. I find the method rather error prone and it has a very limited use: 1) All columns have to be of type string - except the first which must be of type Guid 2) The order of the columns in the table must match the order of the properties of the object type. If they don't, a column could easily be ...


7

is the current implementation 'safe'? No it isn't, because: You did not implement Double-checked locking correctly - you have two fields (_Value and _Loaded) instead of only one. You have added new feature - Invalidate - that invalides the correctness of double-checked locking even if you fix previous problem (by e.g. boxing the value). Lessons to learn ...


6

is there anything else I'm missing that I should be concerned about in a high traffic application? By passing the delegate in the Get method, you're instantiating a delegate object each time you call the property. System.Lazy<T> creates the delegate instance only once.


6

I strongly dislike your AssetCache type. You are literally wrapping a dictionary. The only difference is the type of the exception you are throwing when adding an existing item. Just use a plain old Dictionary<TKey, TAsset> directly. Your ICache interface is also incomplete. Of what use is an interface that lets you destroy data without being able to ...


6

I want it to be generic so I can easily add new links to the chain (like adding a new kind of section between Biome and Landform for example) Generics work fine for simple generic data structures (like a List). Your "generic" data structure is actually a very special one which will not be used outside of your model. It is more a try to extract the ...


5

It will be more efficient to call .ToList() just once: // ... var list = enumerableObjects.Select(stringFieldFunction).ToList(); while (list.Any(currentString => string.Equals(currentString, uniqueString, StringComparison.InvariantCultureIgnoreCase))) { uniqueString = originalString + "_" + duplicateCount++; }


5

private string GetUniqueStringForModel<T>( string originalString, IQueryable<T> databaseQueryable, Expression<Func<T, string>> stringFieldFunction ) where T : class { var lowerBound = originalString.ToLowerInvariant(); var upperBound = originalString.ToUpperInvariant() + "z"; var candidates = new HashSet<...


5

I would suggest using the new() generic type constraint. Using Activator means you can match a private constructor which you would no longer be the case with the generic constraint but you would be able to catch incorrect use at compile time. Currently, if I tried to use a class like: public class Foo { public Foo(string bar, double baz) { /* ... */ }...


5

Why not wrap a Lazy<T> and then lazy load the Lazy<T> in your Get public class MyLazy { private object lazy; private object _Lock = new object(); public T Get<T>(Func<T> factory) { if (lazy == null) { lock (_Lock) { if (lazy == null) { lazy = new Lazy<T>(...


5

public static IEnumerable<AttributeCollection<T>> EnumerateCustomAttributes<T>(this MemberInfo member) where T : Attribute I'm surprised that this method doesn't have a docstring, and I don't find its name very descriptive. What differentiates this from GetCustomAttributes is that it inherits, so I'd expect a name like ...


4

public interface IFaker<T> where T : class { Faker<T> GetFaker(); } I don't think you need this interface and the additional call to GetFaker. Instead you can derive your class from the Faker<T> and use the constructor to set everything up: public class DogFaker : Faker<Dog> { public DogFaker() { RuleFor(dog =>...


4

Yes I think your usage of unique_ptr is correct. The only recommendation I would make is to use make_unique for construction: explicit Queue(std::size_t space) : q_ptr(std::make_unique<T[]>(space)), capacity(space) {} This answer provides motivation for its use. I've also used explicit to avoid implicit conversion from size_t to Queue. And ...


4

I'd recommend avoiding returning the value from pop. If T's copy constructor throws, you lose the data. So I would introduce T top() const and use it along with non-returning void pop(). This approach is used in STL containers which have a pop method. Also calling pop on empty list leads to UB. In my opinion, a linked list based on std::unique_ptr looks more ...


4

I would not recommend this pattern. I see no compelling reason to encumber NetworkRequestError (and presumably every other Error enumeration throughout your project) with this finish cruft. So, instead of: enum NetworkRequestError: Error { case hostNotAvailable case accountNotAvailable func finish<T>() -> Result<T, ...


4

I found something that looks really weird to me for (i = 0; i < dictionary.Dictionary.Count; i++) { maxValue = dictionary.Dictionary.First(); for (j = 1; j < dictionary.Dictionary.Count; j++) { var valuePair = dictionary.Dictionary.ElementAt(j); if (valuePair.Value > maxValue.Value) ...


4

var firstDate = (DateTime)datePropertyInfo.GetValue(stateModels.First()); if (firstDate < dateFrom) dateFrom = new DateTime(firstDate.Year, firstDate.Month, firstDate.Day, firstDate.Hour, 0, 0, DateTimeKind.Utc); You take the first stateModel, but what if they are not ordered by date? for (itemPointer = itemPointer; itemPointer < ...


4

Alternative approach Instead of using attributes and reflection, I'd go for a set of generic methods that take type-specific date and value-selector methods as parameters. First, a method that returns items grouped by time segment: public static IEnumerable<TimeSegmentItems<T>> GetItemsPerTimeSegment( IEnumerable<T> items, ...


4

I will focus on.. Creating custom objects with custom properties using generics Which you do in this method.. public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>() where TCity : City, new() where TBuilding : Building, new() { TCity city = new TCity(); IEnumerable<TBuilding> ...


3

I would rather see 2 static methods to convert a CSV to a collection of objects, and vice versa. A constructor should be as quick as possible to construct a new instance. You violate this with the call to BuildObject(). In GetCsvString, the variable ln should be a StringBuilder since you alter in within the innermost foreach loop. The inner loop could ...


3

Because I'm delightfully (or annoyingly) pedantic, I rather like having interfaces so that I can mock them for testing purposes. I also changed the constructor to be internal since I don't think outside assembly callers need to be calling it except through the .Create() API: IFingerprintBuilder.cs public interface IFingerprintBuilder<T> { ...


3

With the feedback that (my brain) could understand I've came to this for now. (I think) I literally copied the locking structure of Lazy, thread-safe Singleton Included adding the volatile keyword for the _Loaded check Moved the generic definition to the class type. Adding a bit more boilerplate code on the advantage of more type safety and no-boxing Added ...


3

Don't throw Exception It's recommended to throw a specific type of exception, and avoid the overly general Exception. Think twice before using checked exceptions The addRoot method throws Exception when a root node already exists in the tree. This is a checked exception, which means that callers must catch it and handle it. This method is only called by ...


Only top voted, non community-wiki answers of a minimum length are eligible