discrepancy in code docs

Posts   
 
    
Posts: 1255
Joined: 10-Mar-2006
# Posted on: 11-Feb-2017 16:39:04   

Using self servicing, look at the following code


var uow = new UnitOfWork();
uow.AddForSave(someEntity, .......

as you press the comma and the intellisense prompts for the next parameter it says it is boolean for 'recursive' save. Defaults to false. Quick look at XML comments shows that:


    /// <summary>Adds the passed in entity for saving.</summary>
    /// <param name="entityToSave">The entity to save via this unit of work</param>
    /// <param name="recurse">When true, it will save all dirty objects referenced (directly or indirectly) by this entity also.</param>
    /// <returns>true if the entity is accepted, false if the entity is rejected (already added for a similar action)</returns>

Now, same code, but casting as an IUnitOfWorkCore which it implements:


var uow = (IUnitOfWorkCore) new UnitOfWork();
uow.AddForSave(someEntity, .......

as you press the comma and the intellisense prompts for the next parameter it says it is boolean for 'refetcing' the entity. Defaults to false. Quick look at XML comments shows that:


    /// <summary>
    /// Adds the passed in entity for saving. No refetching will be applied. Save is recursive.
    /// </summary>
    /// <param name="toSave">The entity to save via this unit of work</param>
    /// <param name="refetch">When true, it will refetch the entity saved after the save action.</param>
    /// <returns>true if the entity is accepted, false if the entity is rejected (already added for a similar action)</returns>

We use the interface mostly. So confused by this we tested and it appears the option is for 'recursive' save as it appears to always be refetched.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 12-Feb-2017 10:27:45   

It's a design flaw in the Unit of work classes. We needed an interface which matched both the one for adapter and selfservicing, and for adapter we needed 'refetch'. For selfservcing that's not needed. The implementation of the interface on selfservicing's version:



        /// <summary>
        /// Adds the passed in entity for saving. No refetching will be applied. Save is recursive.
        /// </summary>
        /// <param name="toSave">The entity to save via this unit of work</param>
        /// <param name="refetch">When true, it will refetch the entity saved after the save action.</param>
        /// <returns>true if the entity is accepted, false if the entity is rejected (already added for a similar action)</returns>
        void IUnitOfWorkCore.AddForSave(IEntityCore toSave, bool refetch)
        {
            // refetch is ignored as it's implicit.
            this.AddForSave(toSave as IEntity, true);
        }

The interface method defines 'refetch' but for selfservicing this is always the case (it refetches the entity when you touch a property in the entity if it's 'outofsync'). As the selfservicing's IUnitOfWorkCore implementation needed this method implemented we opted for this.

I know it can be confusing, but keep in mind that the method IUnitOfWorkCore.AddForSave() is a member of the interface. It can differ from the one on the class itself. Calls to the IUnitOfWorkCore variant are always recursive, it's part of its signature: 'Save is recursive'.

In hindsight we should have named the method differently.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1255
Joined: 10-Mar-2006
# Posted on: 13-Feb-2017 04:04:53   

Wow, confusing is right.

It can differ from the one on the class itself. Calls to the IUnitOfWorkCore variant are always recursive, it's part of its signature: 'Save is recursive'.

If I have an IUnitOfWorkCore interface and I call AddForSave on it.....is it recursive or not?

Is this what you are saying?

If it is implemented by SelfServicing, it is not recursive unless I pass true to it....?? If it is implemented by Adapter, it is always recursive??

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 13-Feb-2017 09:50:29   

WayneBrantley wrote:

Wow, confusing is right.

It can differ from the one on the class itself. Calls to the IUnitOfWorkCore variant are always recursive, it's part of its signature: 'Save is recursive'.

If I have an IUnitOfWorkCore interface and I call AddForSave on it.....is it recursive or not? Is this what you are saying?

It's always recursive, it's says so even in the documentation of the method.

If it is implemented by SelfServicing, it is not recursive unless I pass true to it....?? If it is implemented by Adapter, it is always recursive??

No, it's the same on both: it's always recursive. The interface contract states the method is always recursive, so it is always recursive. The second parameter, 'refetch', is ignored for selfservicing (refetch is always on, as it's part of the entity), and it's used on adapter.

It's implemented explicitly so the method isn't silently matching the one in the selfservicing UnitOfWork, but in hindsight we should have named it differently.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1255
Joined: 10-Mar-2006
# Posted on: 14-Feb-2017 04:23:02   

It's always recursive, it's says so even in the documentation of the method.

Actually, it says in the documentation of the interface that recursive is optional!

  /// <param name="recurse">When true, it will save all dirty objects referenced (directly or indirectly) by this entity also.</param>

This is what I was saying in title of this thread - discrepancy in code docs. The interface docs says the parameter is for 'recurse'...

No, it's the same on both: it's always recursive. The interface contract states the method is always recursive, so it is always recursive. The second parameter, 'refetch', is ignored for selfservicing (refetch is always on, as it's part of the entity), and it's used on adapter.

Ok, then the docs on the interface need to be changed to indicate that it is for refetch, not recurse?

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 14-Feb-2017 10:11:01   

I quoted the interface method with the xml docs in a post above (from v5.1), it doesn't mention a recurse param, where did you copy the recurse parameter from? The UnitOfWork.AddForSave method? As that's not the interface method, the interface method is implemented explicitly.

So I think you are referring to another AddForSave() method than I am, namely one is the class method and another is the (explicitly implemented) interface method.

You have the class method: http://referencesource42.llblgen.com/#SD.LLBLGen.Pro.ORMSupportClasses/UnitOfWork/UnitOfWork.cs,321

and you have the interface method: http://referencesource42.llblgen.com/#SD.LLBLGen.Pro.ORMSupportClasses/UnitOfWork/UnitOfWork.cs,1138

Those aren't the same thing, one is a class interface, another is the IUnitOfWork interface. Yes, I know both have the same name, and it should have been better if they would have been named differently, but that's not the case.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1255
Joined: 10-Mar-2006
# Posted on: 16-Feb-2017 01:42:01   

Ok, we may just have an issue with the docs in my version. I understand the name could have been better, but this whole issue was about the docs...

Perhaps you have already corrected this issue in 5.x, so probably a non-issue.

For your reference:


// Decompiled with JetBrains decompiler
// Type: SD.LLBLGen.Pro.ORMSupportClasses.UnitOfWork
// Assembly: SD.LLBLGen.Pro.ORMSupportClasses, Version=4.2.0.0, Culture=neutral, PublicKeyToken=ca73b74ba4e3ff27
packages\SD.LLBLGen.Pro.ORMSupportClasses.4.2.20160905\lib\net45\SD.LLBLGen.Pro.ORMSupportClasses.dll

UnitOfWork.cs


    /// <summary>Adds the passed in entity for saving.</summary>
    /// <param name="entityToSave">The entity to save via this unit of work</param>
    /// <param name="recurse">When true, it will save all dirty objects referenced (directly or indirectly) by this entity also.</param>
    /// <returns>true if the entity is accepted, false if the entity is rejected (already added for a similar action)</returns>
    public bool AddForSave(IEntity entityToSave, bool recurse)
    {
      return this.AddForSave(entityToSave, (IPredicate) null, recurse);
    }

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 16-Feb-2017 10:06:42   

yes, that's the class method simple_smile The interface method is implemented as well, explicitly, as a second method. I linked to the sourcecode of v4.2 in my previous post, they're 2 different methods simple_smile The one you quote is the class method of UnitOfWork. Recurse is the parameter, and it is indeed for Recurse. Passing false will not traverse the rest of the graph. If you use the interface though, i.e.:


((IUnitOfWorkCore)myUow).AddForSave(e, true);

that's not calling the method you quoted above, but the method of the interface IUnitOfWorkCore which is implemented explicitly as a second method. After all, you're calling the interface method, not the class method in the example line above.

So to recap:

Calls explicitly implemented method: http://referencesource42.llblgen.com/#SD.LLBLGen.Pro.ORMSupportClasses/UnitOfWork/UnitOfWork.cs,1138


((IUnitOfWorkCore)myUow).AddForSave(e, true);

Calls the class method: http://referencesource42.llblgen.com/#SD.LLBLGen.Pro.ORMSupportClasses/UnitOfWork/UnitOfWork.cs,321


myUow.AddForSave(e, true);

Frans Bouma | Lead developer LLBLGen Pro