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.

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 that intuitive. The problem is obviously that you have to - or can - provide a creator function per call to Get<T>(Func<T> creator) and that it is arbitrary, but only the first actually has any effect.

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 classes look something like this:

public class MyClass

{

    private Lazy<string> _MyStringValue;

    // ...



    public MyClass()

    {

        this._MyStringValue = new Lazy<string>(() => {

            var builder = new StringBuilder();

            builder.Append("a");

            // 50 more lines of expensive construction

            return builder.ToString();

        });



        // 100 more lines constructing OTHER lazy stuff

    }

}

Gloss over the details of building up the value; it's just an example. The important point is that you have all this logic here deep in your constructor.

I think there are two things you can do to alleviate this problem:

1. 
   

   Parameterize

   
   
   

   Why put all this logic in the constructor? You're losing a lot of reusablity by doing that anyway. So make these things parameters and construct them elsewhere:

   
   
   

   public class MyClass
   
   {
   
       private Lazy<string> _MyStringValue;
   
       // ...
   
   
   
       public MyClass(Lazy<string> myStringValue)
   
       {
   
           this._MyStringValue = myStringValue;
   
       }
   
   }
   
   

   
   


2. 
   

   You can embed this construction logic in a method, and then pass the method to the Lazy constructor:

   
   
   

   class MyStringValueMaker
   
   {
   
       // Could be an instance method if that's more appropriate.
   
       // This is just for example
   
       public static string MakeValue()
   
       {
   
           var builder = new StringBuilder();
   
           builder.Append("a");
   
           // 50 more lines of expensive construction
   
           return builder.ToString();
   
       }
   
   }
   
   

   
   
   

   And then elsewhere:

   
   
   

   var myClass = new MyClass(new Lazy<string>(MyStringValueMaker.MakeValue));
   
   

   
   

Now suddenly everything is much better organized, more reusable, and simpler to understand.

If that's not what your class originally looked like, well, then I think you'd be better off posting a new question asking for a review on the original class to get ideas about how to improve.

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>(factory);

                }

            }

        }

        return ((Lazy<T>)lazy).Value;

    }

}

Taking advantage of existing features that have been tried and tested instead of trying to roll your own.

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.

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 a warning to remind myself there might be issues

As for the advice "Leave it to the smarter people". That's something I can't work with. I like to learn, I like other people to learn and I prefer a society where people are motivated to fail (against calculated cost) to learn for themselves.

I appreciate that everyone has a different opinion about that, that's okay.

I still not 100% sure if this solves at least the thread-safety problems of the first version, because the conversation went a bit off-topic imo. If anyone that is knowledgable can comment on that I would appreciate it. For the rest; I'm going to try to use this code and see what it does in production and if it causes (practical) problems for my caching of properties.

/// <summary>

/// Warning: might not be as performant (and safe?) as the Lazy<T>, see: 

/// https://codereview.stackexchange.com/questions/207708/own-implementation-of-lazyt-object

/// </summary>

public class MyLazy<T>

{

    private T               _Value;

    private volatile bool   _Loaded;

    private object          _Lock = new object();





    public T Get(Func<T> create)

    {

        if ( !_Loaded )

        {

            lock (_Lock)

            {

                if ( !_Loaded ) // double checked lock

                {

                    _Value   = create();

                    _Loaded = true;

                }

            }

        }



        return _Value;

    } 





    public void Invalidate()

    {

        lock ( _Lock )

            _Loaded = false;

    }

}

is the current implementation 'safe'?

No it isn't, because:

1. You did not implement Double-checked locking correctly - you have two fields (_Value and _Loaded) instead of only one.


2. 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

1. Always prefer well-known implementations (e.g. System.Lazy<T> or System.Threading.LazyInitializer) over your own - thread/process synchronization and cryptography are two heaviest topics to master, do not expect that you will be able to design these things yourself in a day, it may take years to master!


2. Benchmark/profile before you optimize - lock is often good enough and you can try e.g. System.Threading.SemaphoreSlim to speed it up a bit, but beware that it could get even worse - so again: test and measure first, be clever if you need to, be lazy if you can.


3. If you still wish to write your own version then at least remember:
   
   
   
   
   1. Reads and writes can be reordered in any way (unless they depend on each other like e.g. in assignment a = b + c - the order of fetching b and c is not guaranteed, but write to a has to be done after the computation). Be extra cautious when synchronization involves more than one variable! You will likely think that it works because you do things in some order, but that is wrong! The order is not guaranteed across threads!
   
   
   2. 
      volatile only guarantess the order of writes, not that other threads would see them immediately! That is why your Invalidate is incompatible with double-checked locking, it just won't be seen immediately by the threads that did not call it and may actually NEVER be seen, if they never call lock or something that ensures cache-synchronization.
   
   
   3. I am not an expert ;)