|
You should read all the Names in a List and if the List doesn't
contains the SysIdent value then add it.
A DataReader can't possibly add anything to the DB.
All the best,
Dan
|
|
|
|
|
Hi Dan,
I am aware that the datareader can't add the value, it was in there to illustrate what I'm trying to do
I'm not quite with you on your other statement though. Do you mean create a list<>Names = new list type of thing? If so, how?
|
|
|
|
|
Look at Lucs post below. About the logic, he explaind it I'm lazzy.
Create a List<string> add each item to the list in the while(reader.Read()) loop,
close the reader, check if the list contains what you wan't and if not add it.
Or do it direclty from SQL, eg check the number of rows where YourField equals what you want.
If the affected rows is 0 then it doesn't exist and you can add it. It should be faster.
All the best,
Dan
|
|
|
|
|
There is a flaw in the logic, your DB operations are equivalent to this:
foreach(record in records) {
if (record!=whatWeWant) insert(newRecord);
}
where it should be:
bool exists=false;
foreach(record in records) {
if(record==whatWeWant) exists=true;
}
if (!exists) insert(newRecord);
Now the existance can be checked without an explicit loop, try an SQL statement akin to this:
SELECT COUNT(1) FROM dbo.tablename WHERE FullName = @ValueWeSearch
which you should run through an ExecuteScalar() operation; the COUNT function makes it return one number, the number of matching records; when 0 insert, otherwise don't.
[EDIT] fixed typo [/EDIT]
modified 21-Nov-11 11:05am.
|
|
|
|
|
Luc & Dan,
Thank you very much for pointing out the trees 
SqlCommand selectname = new SqlCommand("SELECT COUNT(*) FROM dbo.SysIdent WHERE FullName = @SysIdent", IdentSys);
selectname.Parameters.Add("@SysIdent", SqlDbType.NVarChar, 150);
selectname.Parameters["@SysIdent"].Value = SysIdent;
IdentSys.Open();
selectname.ExecuteNonQuery();
int Count = Convert.ToInt32(selectname.ExecuteScalar());
if (Count == 0)
{
}
|
|
|
|
|
Almost.
1. COUNT(1) is better than COUNT(*) as the latter may get confused by possible NULL fields (in any field of the record) whereas the former really counts matching rows.
2. Your ExecuteNonQuery() is totally redundant.
3. Your Convert.ToInt32() can be simplified to a simple cast using (int)
|
|
|
|
|
Luc Pattyn wrote: 3. Your Convert.ToInt32() can be simplified to a simple cast using
(int)
|
|
|
|
|
Thanks Luc & PIEBALDconsult, as always, your time spent educating us newbs is appreciated
|
|
|
|
|
There are a few changes I would make to your code if it was me developing this. First of all, I would wrap the SqlConnection and SqlCommand statements with using so that they are disposed of (which ensures that Close is called).
I would also change your insert statement to include the SELECT COUNT as part of the where clause. The reason that I would do this is because you are working in a multi-user database, so it's entirely possible that somebody else could be inserting the value between the time you did the SELECT COUNT and the INSERT. When you do checks like this, it's always important to remember the multi-user access nature of the database.
|
|
|
|
|
Hi Pete,
I'm busy fighting with some vba array code right now, but I'll test your suggestions tonight if that's ok.
|
|
|
|
|
That's fine. I should have also mentioned how you would do this - I'm assuming you are using SQL Server 2008 here, which allows you to do something called an UPSERT command. Basically, that's an INSERT and UPDATE command all rolled into one (although you can just use the INSERT part on its own).
The syntax you would use is
MERGE sometable
USING (select something FROM table) ON (value = @value)
WHEN NOT MATCHED THEN INSERT VALUES (...)
|
|
|
|
|
Hi Pete,
I'm not quite sure what you mean by
Pete O'Hanlon wrote: I would also change your insert statement to include the SELECT COUNT as part of the where clause
but was aware of the multi-user problem. My thoughts on overcoming that issue was to split the transactions into 3 parts, Look for name > if found use the associated ID, if not found > insert name > get Autonumber ID. My code (shabby and lumpy as it may be) currently looks like
try
{
string c = System.Environment.MachineName;
string Domain = System.Net.Dns.GetHostName();
ManagementObjectSearcher SerialNo = new ManagementObjectSearcher("root\\CIMV2", "SELECT * FROM_
Win32_SystemEnclosure");
foreach (ManagementObject querySN in SerialNo.Get())
{
string SN = (querySN["SerialNumber"].ToString());
string SysIdent = c + SN + "@" + Domain;
using(SqlConnection IdentSys = new SqlConnection(some connection;connection timeout=30"));
using(SqlCommand selectname = new SqlCommand("SELECT COUNT(1) FROM dbo.SysIdent WHERE_
FullName = @SysIdent", IdentSys));
selectname.Parameters.Add("@SysIdent", SqlDbType.NVarChar, 150);
selectname.Parameters["@SysIdent"].Value = SysIdent;
IdentSys.Open();
int Count = (int)(selectname.ExecuteScalar());
if (Count == 0)
{
using(SqlConnection InsertSys = new SqlConnection(some other connection_;connection
timeout=30"));
using(SqlCommand newname = new SqlCommand("INSERT INTO dbo.SysIdent(FullName)_
VALUES(@FullName)", InsertSys);
newname.Parameters.AddWithValue("@FullName", SysIdent);
InsertSys.Open();
newname.ExecuteNonQuery();
InsertSys.Close();
}
IdentSys.Close();
using (SqlConnection selectID = new SqlConnection(another connection;connection timeout=30");
SqlCommand GetID = new SqlCommand("SELECT * FROM dbo.SysIdent WHERE FullName = @SysIdent", selectID);
GetID.Parameters.Add("@SysIdent", SqlDbType.NVarChar, 150);
GetID.Parameters["@SysIdent"].Value = SysIdent;
selectID.Open();
SqlDataReader GetmyID = GetID.ExecuteReader();
while (GetmyID.Read())
{
int myID = (int)(GetmyID["ID"]);
Console.WriteLine(myID);
Console.ReadLine();
int mySysId = myID;
if (!EventLog.SourceExists("mylog")) EventLog.CreateEventSource("mylog", "Application");
EventLog.WriteEntry("mylog", "ID" + mySysId + " allocated");
}
}
}
catch (Exception f)
{
if (!EventLog.SourceExists("mylog")) EventLog.CreateEventSource("mylog", "Application");
EventLog.WriteEntry("mylog", "Unable to allocate SysID" + f.ToString());
}
What are your thoughts?
On more-or-less the same issue, given that the end product will be a windows service, what would be the best way to ensure that the ID that is given on the first run remains permanently static. i.e, it will use the same ID when it runs timed_event_1 every 12 hours and the same ID for timed_event_2 which runs every 3 minutes?
|
|
|
|
|
Well, you have a few problems in there. First of all, you need to wrap the code that runs inside the using statements in a { } block so that it is all covered. Basically, using internally maps to a try/finally with the finally part triggering a Dispose, which is why it's good practice to use it on disposable items where possible. (This also explains why you can't use using on a non IDisposable item, as it actually does something like this in the finally part):
((IDisposable)myObject).Dispose(); Secondly, the idea I was talking about was to use the MERGE command as I explained in my followup post. As you haven't provided details on which DB you are using, I have assumed that you are using SQL Server 2008. Alternatively, you could use a command like this:
INSERT INTO dbo.SysIdent(FullName)
SELECT @FullName
WHERE NOT EXISTS (Select FullName FROM dbo.SysIdent WHERE FullName = @FullName) (Note that I've just quickly knocked this statement together in the message window - it might need some refinement). You could return the SCOPE_IDENTITY at this point following the insert to get the auto number that you just added in. Now, if I was being really defensive I would refine this idea to perform the select from SysIdent to see if I've already added the item (and return the id at this point), and if I hadn't added it, I would then perform the insert I demonstrate here. To make my job really easy, I'd put this all inside a stored procedure, so all my C# code would need to do is call the stored procedure to get this value back - don't make your data layer code more complicated than it needs to be.
CCodeNewbie wrote: On more-or-less the same issue, given that the end product will be a windows
service, what would be the best way to ensure that the ID that is given on the
first run remains permanently static. i.e, it will use the same ID when it runs
timed_event_1 every 12 hours and the same ID for timed_event_2 which runs every
3 minutes?
As you haven't said what these events are, I can't give a definitive answer, but I would probably have a property of the class that manages each one of these events which contains the number. It's that simple.
|
|
|
|
|
Coding in VS2005 - .NET2 on an XP SP3 box connecting to SQL Express 2005 (although I might go with MySQL on a Debian box as I assume the db is going to fill quite quickly (see timed_event_2)
timed_event_1 runs every 12 (might change this to 24) hours and collects 'static' info e.g. make,model,serial no., OS, Service Pack, CPU make & Ghz, RAM capacity total - slots in use - capacity per slot, host name/workgroup/domain, internal IP - subnet - gateway - DNS1 & 2, DHCP enabled/static IP, DHCP server, external IP, installed programs
timed_event_2 runs every 3 minutes collecting CPU usage, RAM usage, Disk metrics, avg. ping to gateway, avg. ping to extrenal IP address, running process, open connections (a la netstat -anvb)
The app also collects entries made to the Application, System & Security logs and uploads them to SQL.
Sorry 'bout wrapping, slotted the 'usings' in as I was posting - added them and the braces to the code now.
My SQL db has stored procedures & xp_command turned off & they need to stay that way. All data going in has to come from the code.
Do you mean I should declare some kind of private const in the Initialize Component() area of the service startup and insert the identifier retrospectively? How would I do that? Please bear in mind that I have only been working with C# for ~2 months so please indulge me with simple answers
|
|
|
|
|
I still can't see from this where your system identifier exists in this structure. You have told me what data you are collecting here, but you haven't linked the information together - how does the system identifier figure in all of this? By introducing this concept of timed_event_1 and timed_event_2, you have muddied the waters for me, so I cannot help any further because I could end up sending you completely the wrong way.
|
|
|
|
|
Create Identifying name
string c = System.Environment.MachineName;
string Domain = System.Net.Dns.GetHostName();
ManagementObjectSearcher SerialNo = new ManagementObjectSearcher("root\\CIMV2", "SELECT * FROM_
Win32_SystemEnclosure");
foreach (ManagementObject querySN in SerialNo.Get())
{
string SN = (querySN["SerialNumber"].ToString());
string SysIdent = c + SN + "@" + Domain;
Check if ID exists by looking for Identifying name - string SysIdent
using(SqlCommand selectname = new SqlCommand("SELECT COUNT(1) FROM dbo.SysIdent WHERE_
FullName = @SysIdent", IdentSys));
If name isn't found, insert it into the table
using(SqlCommand newname = new SqlCommand("INSERT INTO dbo.SysIdent(FullName)_
VALUES(@FullName)", InsertSys);
Reconnect to table to get the ID number associated with the name
SqlCommand GetID = new SqlCommand("SELECT * FROM dbo.SysIdent WHERE FullName = @SysIdent", selectID);
set the system's id
int mySysId = myID;
|
|
|
|
|
I got that already - but how does this connect to your timed_event... items? I'm trying, but I can't see what they have to do with this. Take a step back and explain in simple terms what you are trying to achieve with the application - don't dive into implementation details, tell me what problem your application is trying to solve and we'll go from there.
A question - is the system name just meant to be retrieved once per machine? Are you attempting to put the service onto individual machines and that's what the above code is to identify, has the system previously been registered or not (and register it if it hasn't)?
|
|
|
|
|
The purpose of the application is to collect various metrics for individual machines and write those metrics to a SQL table.
In order:-
Service is installed via a msi per individual machine
Service auto-starts and logs the start in the Application & System logs
If this is the first time the code runs, the system queries the 'identity' table for its ID by looking for the id number associated with it name, adds its name to the table if it isn't there already then retrieves the ID number associated with its name and uses that ID 'forever'
* is there a way to set this ID in the code or does it need to get its ID every time?( I would prefer not to use a "cookie" approach nor to write a text file to a file).
on_timed_event_1 fires up
uses the ID allocated above to write the various metrics to the various tables
on_timed_event_2 fires up
uses the ID allocated above to write the various metrics to the various tables
"is the system name just meant to be retrieved once per machine?" - no/yes. The machine uses its system name to identify itself. Once that name is in the table and the system's id has been allocated it uses that id for every transaction afterwards(the id is used as the foreign key in all the other tables)
"Are you attempting to put the service onto individual machines and that's what the above code is to identify" - yes
"has the system previously been registered or not (and register it if it hasn't)?" - exactly
Is this explanation a bit better?
|
|
|
|
|
Right - that's fairly straighforward then. OK, how I would structure this (very loosely):
I would have a manager class which was responsible for triggering the different timed events. This class would also be responsible for triggering the insert/lookup into the database of the system identifier (if I was doing this, I would put this in a separate data layer which could easily be mocked for testing purposes, and I would have the data access happen on a background thread in order not to slow down the startup of the service too much).
I would keep the identifier as a property in the manager class. Now, the manager class would be a member of the service class, and I would instaniate it in the OnStartup event (triggering the system identification).
I hope that this gives you some ideas.
|
|
|
|
|
Pete, I'm sorry but you have lost me, I am insufficiently advanced to fully understand what you are suggesting.
Are you suggesting that I created a second .cs or that I create classes in the same namespace? e.g.
using this;
using that;
namespace a
{
class MyService
{
public MyService()
{
InitializeComponent();
//do stuff
}
}
class Manager
{
public class Manager()
{
// do the get/set id thing
}
}
class ServiceStart
{
protected override void OnStart(string[]args)
{
// create event logs and EnableRaisingEvents stuff
}
}
}
If you would like me to I'll happily post the code I have adapted.
|
|
|
|
|
Not quite. I would create a second class - this does nothing with regards to service management, and it would look something like this:
public class TimedEventManager
{
private BackgroundWorker bw = new BackgroundWorker();
public void Initialise()
{
bw.DoWork += CheckForId();
bw.RunWorkerAsync(null);
}
private void CheckForId(object sender, DoWorkEventArgs e)
{
bw.DoWork -= CheckForId();
bw.Dispose();
}
public int Id { get; set; }
}Then, in your service class, I would create an instance of this:
private TimedEventManager timedEvent;
protected override OnStart(string[] args)
{
timedEvent = new TimedEventManager();
timedEvent.Initialise();
}And that's it. That's how you hook it together.
|
|
|
|
|
Hiya Pete & thank you for helping me out, it is appreciated.
Sorry if I am being thick but I still can't get the SysID to carry across so that I can use it repetitively.
I would like to post my code so that you could look at it and tell me what I am doing wrong but there is a LOT of it and popping it in a forum message makes it hard to read.
Suggestions?
|
|
|
|
|
Put it somewhere that I can download it from and I may be able to take a look at it - no promises at the moment.
|
|
|
|
|
I'll try and do it tonight or, failing that, tomorrow morning-ish. Thanks Pete.
|
|
|
|
|
No problem, but just be aware that I tend not to spend much time on CP at weekends as I like to spend time with the family, and I don't have email notifications turned on because my inbox used to get overwhelmed by emails.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
