Code Review Asked by BKS on December 8, 2021
I have the following method that is responsible for retrieving an Item
from the database. There will be multiple threads instantiating this class, so I want to make sure that the operation will be thread-safe. As a result, I have added a lock around it, however since class only contains instance variables (and no static variables), is adding a lock necessary for this scenario?
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlClient;
namespace My.Library
{
public class MyDataProvider : IMyDataProvider
{
private object sync = new object();
private string connectionString;
public MyDataProvider(string connString)
{
connectionString = connString;
}
public Item GetItem(Guid itemId)
{
lock (sync)
{
Item item = new Item();
string sqlQuery = @"SELECT ItemId, ItemName FROM Item WHERE ItemId = @itemId";
using SqlConnection connection = new SqlConnection(connectionString);
SqlCommand command = new SqlCommand(sqlQuery, connection);
command.Parameters.AddWithValue("@itemId", itemId);
connection.Open();
SqlDataReader reader = command.ExecuteReader();
while (reader.Read())
{
item.ItemId = reader["ItemId"] == DBNull.Value ? Guid.Empty : (Guid)reader["ItemId"];
item.ItemName = reader["ItemName"] == DBNull.Value ? null : reader["ItemName"];
}
return item;
}
}
}
}
According to information you provided it's not necessary to wrap it with lock.
Let's consider situation you described. You wrote:
There will be multiple threads instantiating this class
So there will be one instance for each thread. There's no synchronisation needed, because only one thread will be accessing your class at a time.
Other scenarios:
Your implementation should be fine.
Answered by Karol Miszczyk on December 8, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP