Skip to content

Commit 8590d46

Browse files
JeremyCaneyJeremyCaney
committed
Moved parameters outside of try/catch blocks
There doesn't appear to be a strong benefit to wrapping the `SqlCommand` and `Parameters` in the `try/catch` statement, and by moving them to the main section, we a) reduce nesting, b) are able to reduce comment blocks (since the code within is much smaller and easier to read), and c) keep related code close to one another (i.e., the definition of the `SqlConnection`, `SqlCommand`, and `SqlParameter` objects). Note: I have read concerns on e.g. Stack Overflow that if a connection cannot be established with the server or the database, then the exception will be thrown at the point where the `SqlConnection` or `SqlCommand` were initialized, not where the connection was `.Open()`ed, or the command was executed. I wasn't able to reproduce this, however; missing server, missing databases, and missing objects all happened on one of those two methods, and were thus caught by the `catch` statement—and, perhaps more importantly, the `finally` statement. As such, there doesn't appear to be any benefit to initializing the `SqlConnection`, `SqlCommand`, or `SqlParameter` objects outside of the `try/catch` blocks.
1 parent 07ea524 commit 8590d46

1 file changed

Lines changed: 90 additions & 136 deletions

File tree

‎OnTopic.Data.Sql/SqlTopicRepository.cs‎

Lines changed: 90 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -84,27 +84,20 @@ public override Topic Load(string? topicKey = null, bool isRecursive = true) {
8484

8585
command.CommandType = CommandType.StoredProcedure;
8686

87+
/*------------------------------------------------------------------------------------------------------------------------
88+
| Establish query parameters
89+
\-----------------------------------------------------------------------------------------------------------------------*/
90+
command.AddParameter("TopicKey", topicKey);
91+
command.AddOutputParameter();
92+
93+
/*------------------------------------------------------------------------------------------------------------------------
94+
| Process database query
95+
\-----------------------------------------------------------------------------------------------------------------------*/
8796
try {
8897

89-
/*----------------------------------------------------------------------------------------------------------------------
90-
| Open connection
91-
\---------------------------------------------------------------------------------------------------------------------*/
9298
connection.Open();
93-
94-
/*----------------------------------------------------------------------------------------------------------------------
95-
| Establish query parameters
96-
\---------------------------------------------------------------------------------------------------------------------*/
97-
command.AddParameter("TopicKey", topicKey);
98-
command.AddOutputParameter();
99-
100-
/*----------------------------------------------------------------------------------------------------------------------
101-
| Populate topics
102-
\---------------------------------------------------------------------------------------------------------------------*/
10399
command.ExecuteNonQuery();
104100

105-
/*----------------------------------------------------------------------------------------------------------------------
106-
| Process return value
107-
\---------------------------------------------------------------------------------------------------------------------*/
108101
topicId = command.GetReturnCode();
109102

110103
}
@@ -145,30 +138,19 @@ public override Topic Load(int topicId, bool isRecursive = true) {
145138
};
146139
var reader = (SqlDataReader?)null;
147140

148-
try {
141+
/*------------------------------------------------------------------------------------------------------------------------
142+
| Establish query parameters
143+
\-----------------------------------------------------------------------------------------------------------------------*/
144+
command.AddParameter("TopicID", topicId);
145+
command.AddParameter("DeepLoad", isRecursive);
149146

150-
/*----------------------------------------------------------------------------------------------------------------------
151-
| Open connection
152-
\---------------------------------------------------------------------------------------------------------------------*/
147+
/*------------------------------------------------------------------------------------------------------------------------
148+
| Process database query
149+
\-----------------------------------------------------------------------------------------------------------------------*/
150+
try {
153151
connection.Open();
154-
155-
/*----------------------------------------------------------------------------------------------------------------------
156-
| Establish query parameters
157-
\---------------------------------------------------------------------------------------------------------------------*/
158-
command.AddParameter("TopicID", topicId);
159-
command.AddParameter("DeepLoad", isRecursive);
160-
161-
/*----------------------------------------------------------------------------------------------------------------------
162-
| Execute query/reader
163-
\---------------------------------------------------------------------------------------------------------------------*/
164-
Debug.WriteLine("SqlTopicRepository.Load(): ExecuteNonQuery [" + DateTime.Now + "]");
165152
reader = command.ExecuteReader();
166-
167-
/*----------------------------------------------------------------------------------------------------------------------
168-
| Construct topic graph from reader
169-
\---------------------------------------------------------------------------------------------------------------------*/
170-
topic = reader.LoadTopicGraph();
171-
153+
topic = reader.LoadTopicGraph();
172154
}
173155

174156
/*------------------------------------------------------------------------------------------------------------------------
@@ -226,29 +208,19 @@ public override Topic Load(int topicId, DateTime version) {
226208

227209
command.CommandType = CommandType.StoredProcedure;
228210

229-
try {
211+
/*------------------------------------------------------------------------------------------------------------------------
212+
| Establish query parameters
213+
\-----------------------------------------------------------------------------------------------------------------------*/
214+
command.AddParameter("TopicID", topicId);
215+
command.AddParameter("Version", version);
230216

231-
/*----------------------------------------------------------------------------------------------------------------------
232-
| Open connection
233-
\---------------------------------------------------------------------------------------------------------------------*/
217+
/*------------------------------------------------------------------------------------------------------------------------
218+
| Process database query
219+
\-----------------------------------------------------------------------------------------------------------------------*/
220+
try {
234221
connection.Open();
235-
236-
/*----------------------------------------------------------------------------------------------------------------------
237-
| Establish query parameters
238-
\---------------------------------------------------------------------------------------------------------------------*/
239-
command.AddParameter("TopicID", topicId);
240-
command.AddParameter("Version", version);
241-
242-
/*----------------------------------------------------------------------------------------------------------------------
243-
| Execute query/reader
244-
\---------------------------------------------------------------------------------------------------------------------*/
245222
reader = command.ExecuteReader();
246-
247-
/*----------------------------------------------------------------------------------------------------------------------
248-
| Construct topic graph from reader
249-
\---------------------------------------------------------------------------------------------------------------------*/
250-
topic = reader.LoadTopicGraph(false);
251-
223+
topic = reader.LoadTopicGraph(false);
252224
}
253225

254226
/*------------------------------------------------------------------------------------------------------------------------
@@ -357,68 +329,48 @@ public override int Save([NotNull]Topic topic, bool isRecursive = false, bool is
357329
| Establish database connection
358330
\-----------------------------------------------------------------------------------------------------------------------*/
359331
var connection = new SqlConnection(_connectionString);
360-
var command = (SqlCommand?)null;
361-
362-
try {
363-
364-
/*----------------------------------------------------------------------------------------------------------------------
365-
| Update relations
366-
\---------------------------------------------------------------------------------------------------------------------*/
367-
connection.Open();
332+
var procedureName = topic.Id > 0? "CreateTopic" : "UpdateTopic";
333+
var command = new SqlCommand(procedureName, connection) {
334+
CommandType = CommandType.StoredProcedure
335+
};
368336

369-
/*----------------------------------------------------------------------------------------------------------------------
370-
| Establish command type (insert or update)
371-
\---------------------------------------------------------------------------------------------------------------------*/
372-
var procedureName = topic.Id > 0? "CreateTopic" : "UpdateTopic";
373-
command = new SqlCommand(procedureName, connection) {
374-
CommandType = CommandType.StoredProcedure
375-
};
337+
/*------------------------------------------------------------------------------------------------------------------------
338+
| Set version
339+
\-----------------------------------------------------------------------------------------------------------------------*/
340+
var version = DateTime.Now;
376341

377-
/*----------------------------------------------------------------------------------------------------------------------
378-
| SET VERSION DATETIME
379-
\---------------------------------------------------------------------------------------------------------------------*/
380-
var version = DateTime.Now;
342+
/*------------------------------------------------------------------------------------------------------------------------
343+
| Establish query parameters
344+
\-----------------------------------------------------------------------------------------------------------------------*/
345+
if (topic.Id != -1) {
346+
command.AddParameter("TopicID", topic.Id);
347+
}
348+
else if (topic.Parent != null) {
349+
command.AddParameter("ParentID", topic.Parent.Id);
350+
}
351+
command.AddParameter("Version", version);
352+
command.Parameters.AddWithValue("@Attributes", attributes);
353+
if (topic.Id != -1) {
354+
command.AddParameter("DeleteRelationships", true);
355+
}
356+
command.AddParameter("ExtendedAttributes", extendedAttributes);
357+
command.AddOutputParameter();
381358

382-
/*----------------------------------------------------------------------------------------------------------------------
383-
| Establish query parameters
384-
\---------------------------------------------------------------------------------------------------------------------*/
385-
if (topic.Id != -1) {
386-
command.AddParameter("TopicID", topic.Id);
387-
}
388-
else if (topic.Parent != null) {
389-
command.AddParameter("ParentID", topic.Parent.Id);
390-
}
391-
command.AddParameter("Version", version);
392-
command.Parameters.AddWithValue("@Attributes", attributes);
393-
if (topic.Id != -1) {
394-
command.AddParameter("DeleteRelationships", true);
395-
}
396-
command.AddParameter("ExtendedAttributes", extendedAttributes);
397-
command.AddOutputParameter();
359+
/*------------------------------------------------------------------------------------------------------------------------
360+
| Process database query
361+
\-----------------------------------------------------------------------------------------------------------------------*/
362+
try {
398363

399-
/*----------------------------------------------------------------------------------------------------------------------
400-
| Execute query
401-
\---------------------------------------------------------------------------------------------------------------------*/
364+
connection.Open();
402365
command.ExecuteNonQuery();
403366

404-
/*----------------------------------------------------------------------------------------------------------------------
405-
| Process return value
406-
\---------------------------------------------------------------------------------------------------------------------*/
407367
topic.Id = command.GetReturnCode();
408368

409369
Contract.Assume<InvalidOperationException>(
410370
topic.Id > 0,
411371
"The call to the CreateTopic stored procedure did not return the expected 'Id' parameter."
412372
);
413373

414-
/*----------------------------------------------------------------------------------------------------------------------
415-
| Add version to version history
416-
\---------------------------------------------------------------------------------------------------------------------*/
417-
topic.VersionHistory.Insert(0, version);
418-
419-
/*----------------------------------------------------------------------------------------------------------------------
420-
| Update relations
421-
\---------------------------------------------------------------------------------------------------------------------*/
422374
PersistRelations(topic, connection, true);
423375

424376
}
@@ -442,6 +394,10 @@ public override int Save([NotNull]Topic topic, bool isRecursive = false, bool is
442394
if (attributes != null) attributes.Dispose();
443395
}
444396

397+
/*------------------------------------------------------------------------------------------------------------------------
398+
| Add version to version history
399+
\-----------------------------------------------------------------------------------------------------------------------*/
400+
topic.VersionHistory.Insert(0, version);
445401

446402
/*------------------------------------------------------------------------------------------------------------------------
447403
| Recurse
@@ -477,31 +433,30 @@ public override void Move(Topic topic, Topic target, Topic? sibling) {
477433
base.Move(topic, target, sibling);
478434

479435
/*------------------------------------------------------------------------------------------------------------------------
480-
| Move in database
436+
| Establish database connection
481437
\-----------------------------------------------------------------------------------------------------------------------*/
482438
var connection = new SqlConnection(_connectionString);
483-
var command = (SqlCommand?)null;
484-
485-
try {
486-
487-
command = new SqlCommand("MoveTopic", connection) {
488-
CommandType = CommandType.StoredProcedure
489-
};
439+
var command = new SqlCommand("MoveTopic", connection) {
440+
CommandType = CommandType.StoredProcedure
441+
};
490442

491-
// Add Parameters
492-
command.AddParameter("TopicID", topic.Id);
493-
command.AddParameter("ParentID", target.Id);
443+
/*------------------------------------------------------------------------------------------------------------------------
444+
| Establish query parameters
445+
\-----------------------------------------------------------------------------------------------------------------------*/
446+
command.AddParameter("TopicID", topic.Id);
447+
command.AddParameter("ParentID", target.Id);
494448

495-
// Append sibling ID if set
496-
if (sibling != null) {
497-
command.AddParameter("SiblingID", sibling.Id);
498-
}
449+
// Append sibling ID if set
450+
if (sibling != null) {
451+
command.AddParameter("SiblingID", sibling.Id);
452+
}
499453

500-
// Execute Query
454+
/*------------------------------------------------------------------------------------------------------------------------
455+
| Process database query
456+
\-----------------------------------------------------------------------------------------------------------------------*/
457+
try {
501458
connection.Open();
502-
503459
command.ExecuteNonQuery();
504-
505460
}
506461

507462
/*------------------------------------------------------------------------------------------------------------------------
@@ -546,22 +501,21 @@ public override void Delete(Topic topic, bool isRecursive = false) {
546501
| Delete from database
547502
\-----------------------------------------------------------------------------------------------------------------------*/
548503
var connection = new SqlConnection(_connectionString);
549-
SqlCommand? command = null;
550-
551-
try {
552-
553-
command = new SqlCommand("DeleteTopic", connection) {
554-
CommandType = CommandType.StoredProcedure
555-
};
504+
SqlCommand? command = new SqlCommand("DeleteTopic", connection) {
505+
CommandType = CommandType.StoredProcedure
506+
};
556507

557-
// Add Parameters
558-
command.AddParameter("TopicID", topic.Id);
508+
/*------------------------------------------------------------------------------------------------------------------------
509+
| Establish query parameters
510+
\-----------------------------------------------------------------------------------------------------------------------*/
511+
command.AddParameter("TopicID", topic.Id);
559512

560-
// Execute Query
513+
/*------------------------------------------------------------------------------------------------------------------------
514+
| Process database query
515+
\-----------------------------------------------------------------------------------------------------------------------*/
516+
try {
561517
connection.Open();
562-
563518
command.ExecuteNonQuery();
564-
565519
}
566520

567521
/*------------------------------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)