Skip to content

Commit baaaf4d

Browse files
committed
Move variables to top of Save()
This not only makes it easier to get a sense of `Save()'s scope of responsibility, but more importantly will allow us to introduce an escape clause for `isDirty` in a future commit, thus preventing the need to do subsequent processing if `!isDirty`. As part of this, renamed `excludeLastModified` to `areAttributesDirty`, which is more descriptive given that it's no longer being used exclusively for tracking `excludeLastModified`. This is more consistent with `areRelationshipsDirty` as well.
1 parent 727eaef commit baaaf4d

1 file changed

Lines changed: 30 additions & 32 deletions

File tree

OnTopic.Data.Sql/SqlTopicRepository.cs

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,36 @@ List<Topic> unresolvedRelationships
332332
\-----------------------------------------------------------------------------------------------------------------------*/
333333
base.Save(topic, isRecursive, isDraft);
334334

335+
/*------------------------------------------------------------------------------------------------------------------------
336+
| Define variables
337+
\-----------------------------------------------------------------------------------------------------------------------*/
338+
var isNew = topic.Id == -1;
339+
var version = new SqlDateTime(DateTime.Now);
340+
var areReferencesResolved = true;
341+
var areRelationshipsDirty = topic.Relationships.IsDirty();
342+
var areAttributesDirty = topic.Attributes.IsDirty(excludeLastModified: true);
343+
var extendedAttributeList = GetAttributes(topic, isExtendedAttribute: true);
344+
var indexedAttributeList = GetAttributes(
345+
topic : topic,
346+
isExtendedAttribute : false,
347+
isDirty : true,
348+
excludeLastModified : !areAttributesDirty
349+
);
350+
351+
/*------------------------------------------------------------------------------------------------------------------------
352+
| Detect whether anything has changed
353+
>-------------------------------------------------------------------------------------------------------------------------
354+
| If no relationships have changed, and no attributes values have changed, and there aren't any mismatched attributes in
355+
| their respective lists, then there isn't anything new to persist to the database, and thus no benefit to executing the
356+
| current command. A more aggressive version of this would wrap much of the below logic in this, but this is just meant
357+
| as a quick fix to reduce the overhead of recursive saves.
358+
\-----------------------------------------------------------------------------------------------------------------------*/
359+
var isDirty =
360+
areRelationshipsDirty ||
361+
areAttributesDirty ||
362+
indexedAttributeList.Any() ||
363+
extendedAttributeList.Any(a => a.IsExtendedAttribute == false);
364+
335365
/*------------------------------------------------------------------------------------------------------------------------
336366
| Establish attribute containers with schema
337367
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -352,18 +382,6 @@ List<Topic> unresolvedRelationships
352382
/*------------------------------------------------------------------------------------------------------------------------
353383
| Add indexed attributes that are dirty
354384
\-----------------------------------------------------------------------------------------------------------------------*/
355-
356-
//Only include `LastModified` and/or `LastModifiedBy` if other attributes in the collection are `IsDirty`, as these are
357-
//automatically generated and don't represent genuine changes to the topic.
358-
var excludeLastModified = !topic.Attributes.IsDirty(excludeLastModified: true);
359-
360-
var indexedAttributeList = GetAttributes(
361-
topic : topic,
362-
isExtendedAttribute : false,
363-
isDirty : true,
364-
excludeLastModified : excludeLastModified
365-
);
366-
367385
foreach (var attributeValue in indexedAttributeList) {
368386

369387
var record = attributes.NewRow();
@@ -380,8 +398,6 @@ List<Topic> unresolvedRelationships
380398
\-----------------------------------------------------------------------------------------------------------------------*/
381399
extendedAttributes.Append("<attributes>");
382400

383-
var extendedAttributeList = GetAttributes(topic, isExtendedAttribute:true);
384-
385401
foreach (var attributeValue in extendedAttributeList) {
386402

387403
extendedAttributes.Append(
@@ -419,14 +435,10 @@ void addUnmatchedAttribute(string key) {
419435
/*------------------------------------------------------------------------------------------------------------------------
420436
| Establish database connection
421437
\-----------------------------------------------------------------------------------------------------------------------*/
422-
var isNew = topic.Id == -1;
423438
var procedureName = isNew? "CreateTopic" : "UpdateTopic";
424439
var command = new SqlCommand(procedureName, connection) {
425440
CommandType = CommandType.StoredProcedure
426441
};
427-
var version = new SqlDateTime(DateTime.Now);
428-
var areReferencesResolved = true;
429-
var areRelationshipsDirty = topic.Relationships.IsDirty();
430442

431443
/*------------------------------------------------------------------------------------------------------------------------
432444
| Handle unresolved references
@@ -440,20 +452,6 @@ void addUnmatchedAttribute(string key) {
440452
areReferencesResolved = false;
441453
}
442454

443-
/*------------------------------------------------------------------------------------------------------------------------
444-
| Detect whether anything has changed
445-
>-------------------------------------------------------------------------------------------------------------------------
446-
| If no relationships have changed, and no attributes values have changed, and there aren't any mismatched attributes in
447-
| their respective lists, then there isn't anything new to persist to the database, and thus no benefit to executing the
448-
| current command. A more aggressive version of this would wrap much of the below logic in this, but this is just meant
449-
| as a quick fix to reduce the overhead of recursive saves.
450-
\-----------------------------------------------------------------------------------------------------------------------*/
451-
var isDirty =
452-
areRelationshipsDirty ||
453-
!excludeLastModified ||
454-
indexedAttributeList.Any() ||
455-
extendedAttributeList.Any(a => a.IsExtendedAttribute == false);
456-
457455
/*------------------------------------------------------------------------------------------------------------------------
458456
| Establish query parameters
459457
\-----------------------------------------------------------------------------------------------------------------------*/

0 commit comments

Comments
 (0)