|
Hi Jacob,
I think you handled the stopping and disposing the wrong way. For one, you should not implement a destructor; normally one implements a Dispose() method that disposes first by providing and calling a Dispose(bool) method, then calls SuppressFinalize. Your Dispose() not calling Dispose(bool) at all, I think your destructor is unreachable. On top of that I see no need for all of it.
This is what I would do:
D1. remove "IDisposable" and all dispose related code from LoadPicture. There is no need for LoadPicture to be disposable, it is not holding on to lots of memory that otherwise may get lost.
D2. implement a better Stop method; all you need is for LoadPicture to really have stopped somehow before you close the Form, and make the Progress/Finished events point to something that does no longer exist.
public void Stop() {
stop = true;
ProgressChanged = null;
Finished = null;
}
This is not waterproof yet, there is a slim chance of Load() to have tested say ProgressChanged (turned out to be not null), the main thread starting to close the form, setting ProgressChanged null, stop true, actually then closing the form, and only then Load calling ProgressChanged.
So you could:
- make stop volatile
- and test both the event and stop in that order (opposite from the Stop order), hence:
if (ProgressChanged != null && !stop) ProgressChanged(dataIndex, ...)
That should take care of it.
I do have more comments:
C1.
you used a strange way to cope with the end of the stream. I would simply use a loop containing
bytesRead += r.GetBytes(0, dataIndex, buffer, dataIndex, bufferSize);
dataIndex += bytesRead;
and exit the loop when bytesRead turns zero.
C2.
your progress reporting seems expensive, in two ways:
a. you switch from int/long to decimal, to cope with small sizes, which would vanish when divided by 1024 or 1024*1024.
b. and you report very frequently, so consecutive reports will not differ by much, probably wasting a lot of CPU cycles, and actually slowing down progress significantly.
I would use a larger data buffer (say 64KB), calculate an integer percentage locally, compare it to the previous value, and only report it on a change of percentage value.
C3.
You don't want your thread (t4) to ever prevent your app from exiting, so it is a background thread. You should tell it so: t4.IsBackground=true; before t4.Start();
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
|
|
|
|
|
I am still trying your example.. Having issues with converting the bytesRead and chanigng the way I read the bytes.
Plus i think bytesRead would never be zero since you are using +=
|
|
|
|
|
Sorry, my mistake; you are right of course. What I meant was:
int newAmount = r.GetBytes(0, dataIndex, buffer, dataIndex, bufferSize);
bytesRead += newAmount;
dataIndex += newAmount;
and exit when newAmount is zero. You probably don't need both bytesRead and dataIndex now.
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
|
|
|
|
|
public void Load()
{
SqlConnection conn = new SqlConnection(AssetConfig.ConnectionString);
SqlCommand cmd = new SqlCommand("SELECT Picture FROM Inventory_Pictures WHERE PictureID=@Tag", conn);
cmd.Parameters.AddWithValue("@Tag", Tag);
try
{
conn.Open();
IDataReader r = cmd.ExecuteReader(CommandBehavior.SequentialAccess);
if (r != null)
{
bool exist = r.Read();
if (exist)
{
long size = r.GetBytes(0, 0, null, 0, 0);
byte[] buffer = new byte[size];
int bufferSize = 65536;
long bytesRead = 0;
decimal prevPercent = 0;
while ((bytesRead < size) && ((size - bytesRead) > bufferSize))
{
if (stop)
break;
long newAmount = r.GetBytes(0, (Int32)bytesRead, buffer, (Int32)bytesRead, bufferSize);
bytesRead += newAmount;
if (newAmount == 0)
break;
decimal FileSize = (size / 1024) / 1024;
decimal CurrentSize = (bytesRead / 1024) / 1024;
if (ProgressChanged != null && !stop)
{
decimal currentPercent = (CurrentSize / FileSize) * 100;
if (currentPercent != prevPercent)
ProgressChanged((int)CurrentSize, (int)FileSize, Math.Round(CurrentSize, 2).ToString(), Math.Round(FileSize, 2).ToString());
prevPercent = currentPercent;
}
}
if (!stop)
{
MemoryStream ms = new MemoryStream(buffer);
if (Finished != null)
Finished(Image.FromStream(ms));
ms.Close();
}
}
r.Close();
}
}
catch (Exception ex)
{
Error.WriteError(ErrorType.SQL, "LoadPicture", ex.ToString());
}
finally
{
if (conn != null)
{
conn.Close();
conn.Dispose();
}
if (cmd != null)
cmd.Dispose();
}
}
Problem is ProgressChanged is apparently being called after frmViewAsset disposes??
[5/24/2010 10:39:13 AM LoadPicture]: System.InvalidOperationException: Invoke or BeginInvoke cannot be called on a control until the window handle has been created.
at System.Windows.Forms.Control.MarshaledInvoke(Control caller, Delegate method, Object[] args, Boolean synchronous)
at System.Windows.Forms.Control.Invoke(Delegate method, Object[] args)
at System.Windows.Forms.Control.Invoke(Delegate method)
at AssetMgmt.frmViewAsset.p_Finished(Image img) in C:\Applications\Windows Applications\AssetMgmt\AssetMgmt\frmViewAsset.cs:line 379
at AssetMgmt.LoadPicture.Load() in C:\Applications\Windows Applications\AssetMgmt\AssetMgmt\Operations\LoadPicture.cs:line 81
It fails on the Finished.. I think only because I can't close the window quick enough during the Progress. Funny thing is I saw the image appear in the picturebox before it finally closed and it still showed this
modified on Monday, May 24, 2010 11:39 AM
|
|
|
|
|
well, now it fails in finished, and rightly so; you basically have:
if (!stop) {
MemoryStream ms = new MemoryStream(buffer); <<< long operation
if (Finished != null) {
Image img=Image.FromStream(ms); <<< long operation
Finished(img);
...
}
and during those long operations another thread (the GUI thread handling your attempt to close down) has ample time to modify variables which you have already tested and passed.
Change it to:
if (!stop) {
MemoryStream ms = new MemoryStream(buffer); <<< long operation
if (Finished != null) {
Image img=Image.FromStream(ms); <<< long operation
if (Finished != null && !stop) Finished(img);
}
...
}
which you may simplify to:
MemoryStream ms = new MemoryStream(buffer); <<< long operation
Image img=Image.FromStream(ms); <<< long operation
if (Finished != null && !stop) Finished(img);
...
if you want (possibly causing some cycles being wasted in some scenario's)
BTW: the same applies to progress; there too, the inner conditional block should be minimal.
This approach isn't waterproof yet, it still is conceivable that the situation changes (variables change value AND form gets closed AND everything gets collected) between the if and the event being called. One extra safety measure would be to add a small delay in the FormClosing event:
change the variables
Thread.Sleep(100);
as the FormClosing will be followed by a FormClosed event, and only then the form will be disposed.
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
|
|
|
|
|
Ok maybe I am just being picky.
chnaged:
if (ProgressChanged != null && !stop)
{
decimal currentPercent = (CurrentSize / FileSize) * 100;
string CurrentMB = Math.Round(CurrentSize, 2).ToString();
string MaxMB = Math.Round(FileSize, 2).ToString();
int Current = (int)CurrentSize;
int Total = (int)FileSize;
if (currentPercent != prevPercent && ProgressChanged != null && !stop)
ProgressChanged(Current, Total, CurrentMB, MaxMB);
prevPercent = currentPercent;
}
if (!stop)
{
MemoryStream ms = new MemoryStream(buffer);
Image img = Image.FromStream(ms);
if (Finished != null && !stop)
Finished(img);
ms.Close();
}
This way like you were saying all the possible long process task are done before calling the event.
I put Thread.Sleep(100) in there and gave it a try. About 1/6 times I got the object disposed error. That only happened if I opened it and managed to IMMEDIATELY close it. (which i doubt users will be as quick as I was when working on this).
|
|
|
|
|
I agree with your code, and I'm surprised it still fails at that rate.
The reason for the sleep (and the imperfection of the approach) is, basically, you would need to use a lock; however I don't like putting a lock inside a FormClosing/FormClosed event, as any bug or behavioral problem in your app would cause it not to close at all, which tends to distress the user very much (no more than an exception that crashes the app though).
You might try one more refinement: inside FormClosing, rather than a single Thread.Sleep(100), do:
for(int i=0; i<5; i++) {
if (!t4.IsAlive) break;
Application.DoEvents();
Thread.Sleep(100);
}
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
|
|
|
|
|
Lol I see where you say DoEvents is evil.. could you elaborate on that for me little bit so I know why?
As for the latest cost I think I might leave it as is. Because the user doesn't really notice the error message, it writes it behind the scenes to a file. Also it isn't happening that often now. Only when you close the form right when its opened.. Meaning you have to be trying to make it break for it to throw the error (which is what I'm doing lol)
I might leave it like it is.. I'm not fond of it not handling it like it should but what else do we do? I mean from what I know about programming it looks like technically it should work!!
The only thing I can think of is something with using the Invoke((Action) (()=> instead of the traditional Control.InvokeRequired. Don't know why but that is the only thing we haven't changed yet lol
|
|
|
|
|
1.
DoEvents() is evil because it adds re-entrancy to your code: pieces of code that were never going to be executed twice at the same time could suddenly run twice. Example:
public void Button_Click(...) {
long-wielding calculation
more long-wielding calculation
}
in the above, the GUI is dead as long as the calculations keep the GUI thread occupied. Inserting one or more DoEvent() calls, causes the message pump to get pumping again; now if you had clicked the same button, the same handler would suddenly run for a second time. And the handler's code was not designed in a thread-safe way as it was never expected to run concurrently. Each run would have its own set of local variables (as they are on stack, there is a stack for each thread), however the object's state, as maintained in the class data, could easily become inconsistent and invalid.
2.
I did not suggest modifying things that look fine; your Invoke (or one of its alternatives), while essential for things to go wrong, is not the cause of the problem.
3.
Jacob Dixon wrote: you have to be trying to make it break for it to throw the error (which is what I'm doing
Trying to break it is part of quality assurance; abuse is a must at that time!
If it were to fail when used normally, now that would really be unacceptable.
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
|
|
|
|
|
hello everyone
I want to know how to specify the type of a variable declared as Object
better:
I have a variable type object, and I want to test whether this object is string it will make me so and so instrution and if this object and int, and so on.
thank you
Here is a snippet incomplet:
public static string Format(string format, object val,int length)
{
LineDefinition LD;
string var;
if (val.GetType() )
var = string.Format(LD.Format , val).PadRight (LD .Length ,' ');
var = string.Format(LD.Format, val).PadLeft (LD.Length, '0');
return var;
|
|
|
|
|
So you have a type that is an object and you want to determine whether or not it can be cast to a particular type. Is that what you are asking? If so (and if it is possible that the value comes in looking like a string), you can use the TryParse method on a particular type to determine whether or not it is of that type. You need to be careful with this approach because you need to carefully order the sequence of tests; this means that if you have the following:
object o = 10; then o could be a short, an int or a long.
If the variable looks like this example, then you can use the is operator to determine the type, e.g.
if (o is int)
{
}
"WPF has many lovers. It's a veritable porn star!" - Josh Smith As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.
My blog | My articles | MoXAML PowerToys | Onyx
|
|
|
|
|
in my case, I will use the second example,
thank you very much for your help, I really need it 
|
|
|
|
|
Dear All,
I am workinf on multi-project enviroment. And this is the error I am receiving while building:
A project with an Output type of Class Library cannot be started directly. In order to debug this project, add an executable project to this solution which references the library project. Set the executable project as the startup project.
I have set up the starting project. There are 2 errors, that is why build is not succeeded, I do not want to change anything right now since I am new at the company. I want to run the project anyway. But somehow the executable is not generated.
I need help, what I could do.
|
|
|
|
|
You can't run a DLL. Change your Startup project in your solution to a project that results in a EXE file.
.45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001
|
|
|
|
|
I know that, what I do not understand is it should generate an executable for a project, I choose that project however somehow it does not generate executable.
|
|
|
|
|
tan_chin wrote: what I do not understand is it should generate an executable for a project
The application type in your projects properties is "Class Library", not a Console or Windows Forms app. You cannot directly execute a Class Library. You either have picked the wrong project type (this does NOT change on it's own!) or you need a second project (Console or Windows Forms app) as a "test bed" to exercize your library.
|
|
|
|
|
tan_chin wrote: There are 2 errors, that is why build is not succeeded, I do not want to change anything right now since I am new at the company.
Why do you not want to fix them - is it somebody elses responsibility to fix an error that you know about? The error is telling you that you can't start a DLL directly; you need an executable to run it. That's what the error is trying to tell you.
Think about it this way. Your DLL exposes methods that need to be called in a certain way using inputs and logic in a particular order. This is what the executable is going to do, so you need to use it to call these methods.
"WPF has many lovers. It's a veritable porn star!" - Josh Smith As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.
My blog | My articles | MoXAML PowerToys | Onyx
|
|
|
|
|
|
Hi,
I'm trying to read an Excel spreadsheet that contains a variable amount of data, names, address, etc. I will always have columns A-Z, but the number of rows will change. I'm trying to determine the last row in the spreadsheet that contains data. I've tried this code, but it returns a very large number:
public int GetNumberOfRows( )
{
if (excelWorksheet == null)
return (-1);
return (excelWorksheet.Rows.Count);
}
Any help is greatly appreciated.
Glenn
|
|
|
|
|
This might give you some inputs, although not absolutely sure.
My signature "sucks" today
|
|
|
|
|
Good Morning,
I am trying to pass some information to a new page, create a string array, and then use that information in additional code in the application. It seems to work fine until the last step, where the array values seem to disappear (goes null). Somewhere between when the new window closes and I am returned to the original code, I lose the information. I have detailed the process in the code below. Please let me know where I am making my mistake. Thank you in advance for your help, Pat
public frmMain(ArrayList isChecked)
{
InitializeComponent();
this.checkedNames = isChecked;
}
private void buttonWhere_Click(object sender, EventArgs e)
{
for (int iItem = 0; iItem <= listViewColumns.Items.Count - 1; iItem++)
{
colNames.Add(listViewColumns.Items[iItem].SubItems[0].Text);
}
Where w = new Where(colNames);
w.ShowDialog();
foreach (string name in checkedNames)
{
}
}
|
|
|
|
|
Make sure that listViewColumns is not getting set to null in the second page.
My signature "sucks" today
|
|
|
|
|
Abhinav,
Thank you for looking at my dilemma. I am posting the second page code below:
for (int iItem = 0; iItem <= checkedListBox1.CheckedItems.Count - 1; iItem++)
{
string s = (checkedListBox1.CheckedItems[iItem].ToString());
checkedNames.Add(s);
}
frmMain m = new frmMain(checkedNames);
this.Close();
I appreciate any other insight that you might have...Best Regards, Pat
|
|
|
|
|
So, in your frmMain, you have an ArrayList. And a button.
You click the button, and create an instance of your second page, a form called "Where". You then show the form using ShowDialog().
In the form, you merrily add to your ArrayList, and hand it to a new instance of your main form.
Your Where form then closes.
What has happened, is that you new frmMain instance goes out of scope. It is then destroyed. Allongh with it's copy of the ArrayList.
You are then back in your original instance of frmMain, just after your ShowDialog call, and you check the ArrayList. Supprise! It's empty!
Of course it's empty! You added all your data to the local instance you created in the Where form.
What you need to do is when you get back to your original instance, get the arrayList reference from the Where form instance, and copy it to the checkdNames arrayList.
Did you know:
That by counting the rings on a tree trunk, you can tell how many other trees it has slept with.
|
|
|
|
|
The previous answer may not make much sense when you read it. It's is very hard to explain without pictures.
Try this: Use a new sheet of paper every time you use the "new" keyword, and write on it what you have added to it.
Step yourself through your code, and see what happens to your pieces of paper!
Did you know:
That by counting the rings on a tree trunk, you can tell how many other trees it has slept with.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
