[RESOLVED] Exclude checked lines from file

[Migeria]

tags.db

asd||$|blah
qwe||$|jack daniels
zxc||$|exam
qaz||$|fail

checkedListBox1 items

asd
qwe
zxc
qaz

if(checkedListBox1->CheckedItems->Count != 0)
{
	String ^ line, ^ toDelete;
	array<String^>^ seperator = {"||$|"}, ^ seperator2 = {"|$|"}, ^ tag, ^ delete;
	for each(line in IO::File::ReadLines("tags.db"))
	{
	        tag = line->String::Split(seperator, StringSplitOptions::None);
// Why this way? If I remove x++, it become's infinite loop
		for(int x = 0; x <= checkedListBox1->CheckedItems->Count-1; x++) 
		{
			Object^ checked = checkedListBox1->CheckedItems[0];
			if (tag[0] == checked->ToString())
			{
				toDelete = String::Concat(toDelete, tag[0],"|$|");
				checkedListBox1->Items->Remove(checkedListBox1->CheckedItems[0]);
			} else {
				toDelete = String::Concat(toDelete, " ","|$|");
			}
		}
		delete = toDelete->String::Split(seperator2, StringSplitOptions::RemoveEmptyEntries);
		Array::Sort(delete);
		if(Array::BinarySearch(delete, tag[0]) < 1)
		{
			IO::File::AppendAllText("tags.db.temp", line+"\n");
		} else {
			IO::File::AppendAllText("tags.db.temp","");
		}
	}
	IO::File::Copy("tags.db.temp", "tags.db", true);
	IO::File::Delete("tags.db.temp");
}

Supposed to do:
Checked items are NOT included when writing to .temp file (includes only line from .db file which are not checked).

(1) In this, if I check ONLY 1st OR 1st and 1 more item from checkedListBox1: unable to exclude 1st line.
(2) If I check first item and 2 others (or more, all), lines are excluded.
In case only 1 item is left in checkedListBox1, I'm back to (1). =/

[Eri523]

Although not long, that code is pretty convoluted (yet again) and it took me half an eon to even get a rough guess of what it does.

I would take a considerably different approach, most likely resulting in code that's both shorter and easier to understand.

I suppose you've already read the file earlier in your program in order to populate the CheckedListBox. At that point I would keep a copy of the file in memory, probably in an array<String ^> returned by File::ReadAllLines() (not ReadLines() because that likely only pays for really large files).

When it comes to writing back the file, I would parallelly iterate over both the items of the CheckeListBox and the the in-memory copy of the file, writing back only those lines whose corresponding CheckedListBox item is not checked. And int index would be perfect for this sort of iteration. I would use a StreamWriter for the writing process.

This would be easy unless you want to have the CheckedListBox sorted while keeping the original, unsorted sequence of lines in the file, and among others that are probably even more important, would have the advantage of allowing to write directly to the target file without the need to create a temporary output file.

Some remarks about the code you have now:

// Why this way? If I remove x++, it become's infinite loop
		for(int x = 0; x <= checkedListBox1->CheckedItems->Count-1; x++)

To be honest, I'm not sure about this either. x is not used inside the loop, and so, without the x++, it would be equivalent to this:

while (checkedListBox1->CheckedItems->Count > 0)

which should work, according to the basic laws of logic. That it doesn't and we don't know why is another proof for the fact that your code is overcomplicated.

		} else {
			IO::File::AppendAllText("tags.db.temp","");
		}

This is redundant and although I don't know how AppendAllText() is implemented, I suspect it's pretty expensive for something that does nothing. In general, I don't think it's really efficient to repeatedly open and close the file for each line you want to write to it.

I don't see a reason behind first concatenating the "keys" of the lines to be deleted into one long string (delete) and then later pick it apart again using Split(). Instead, I would create an empty container right from the start and then add the keys to be deleted to its end. While you can't use an array for that because it's of fixed length, a List<String ^> would perfectly fit that scenario. BTW, you are adding redundant entries here as well that you unnessecarily need to have removed during the Split().

All these concerns would become irrelevant, however, if you switch to the simplified approach I described at the beginning of this post.

[Migeria]

array<String^> ^ lines;
private: System::Void Form1_Load(System::Object^  sender, System::EventArgs^  e) {
	 String ^ file("tags.db");
	 if(!File::Exists(file))
	 {
		 IO::FileInfo ^ duomf = gcnew FileInfo(file);
		 duomf->Create();
		 MessageBox::Show("Nerastas "+file+" failas!\nTodel buvo sukurtas naujas.","Nerastas duomenu Failas");
	 } else {
		 array<String^> ^ tag, ^ seperator = {"||$|"};
		 lines = File::ReadAllLines(file);
		 for each(String^ line in lines)
		 {
			 tag = line->String::Split(seperator, StringSplitOptions::None);
			 checkedListBox1->Items->Add(tag[0]);
		 }
	 }
 }
private: System::Void button3_Click(System::Object^  sender, System::EventArgs^  e) {
	 array<String^> ^ tag, ^ seperator = {"||$|"};
	 array<int>^ indexes;
	 int index = 0;
	 StreamWriter^ sw = gcnew StreamWriter("asd.txt");
	 for each (String ^ line in lines)
	 {
		 tag = line->String::Split(seperator, StringSplitOptions::None);
		 if(!checkedListBox1->GetItemChecked(index))
		 {
			 sw->StreamWriter::WriteLine(line);
		 }
		 index++;
	 }
	sw->Flush();
	sw->Close();
 }

DELETED... and get index into array, I will need to remove them from checkedListBox1 (I can do it with String::Concat and Split(), but maybe there's better way)

[Eri523]

Simple, isn't it?

I would have written some parts of the code differently, but that would not necessarily be an improvement, rather a matter of personal taste. And there's practically always more than one way to achieve a goal in programming. Overall, your code is what I meant.

... and get index into array, I will need to remove them from checkedListBox1 (I can do it with String::Concat and Split(), but maybe there's better way)

I thought about that as well after writing my last post, but at that time I was away from my PC. In my original idea I somehow naively assumed you would close the form after processing the contents of checkedListBox1.

I think the easiest way to achieve this is not to remove elements from checedListBox1 and the lines array at all, but instead rebuild both of them from scratch on the basis of the file because you already have the code for that.

To do that, you would factor out the entire body of the function Form1_Load() into a new separate private member function and add a

checkedListBox1->Items->Clear();

at the beginning of the new function because you can no longer assume checkedListBox1 will be empty when this point in the code is reached. You would then call that new function as the only line in Form1_Load() and as the last line in button3_Click(), after everything you have in there now is done.

As you save the modified data to a different file than you originally loaded it from, the name of the file would need to be passed as a parameter to the new function that builds the lines array and populates checkedListBox1. That's because you of course would need to reload the modified state from the file you just saved at the end of button3_Click().

Some more (rather minor) remarks:

You split the contents of each line into the tag array in button3_Click() but then never use it. Therefore the line that does this and the variables tag and separator can be removed. The same applies to the variable indexes that isn't used at all (didn't that raise a compiler warning?).

		 if(!checkedListBox1->GetItemChecked(index))
		 {
			 sw->StreamWriter::WriteLine(line);
		 }

You don't need to specify the class name when you call object methods. This is only required when calling static methods. You didn't do that when calling CheckedListBox::GetItemChecked() either in the first line of that snippet.

You don't need to explicitly flush the file stream before closing it. The Close() method does that implicitly anyway.

[Migeria]

Oh.. I use tag array in my code I edited it before posting here and didn't see that I left it to be.
As for int array indexes, I was planing to store not checked items indexes for later removal, but now that I successfully compiled your suggestion, I have removed it.
File name will stay the same as in Form1_Load(), again I forgot to edit it. I tested output to different file, so that I wouldn't need to take time copying old files in case of failure.

Thing is I hardly wanted to use C++/CLI when was enlighten about its existence in my code xD But now that I talked about it with my don/lecturer/professor (whichever is correct) I may use it as extra or adjuvant (something in between).
So YAY... I gotta change everything what I did in project with C++, 'cause it will look stupid to have same parts in different code xD

"Using object oriented programming model, also meeting demands:
*...
*... Use only C++ stream functions for input/output operations."
I just noticed it, so I didn't as about it =/ in your opinion I'm allowed to use something like StreamWriter? Or is C++ and C++/CLI two different worlds?

[Eri523]

Or is C++ and C++/CLI two different worlds?

Oh yes, they are. As you already have seen the two can cooperate, but that should be avoided if possible because it causes extra overhead both in writing and executing the code.

*... Use only C++ stream functions for input/output operations."
I just noticed it, so I didn't as about it =/ in your opinion I'm allowed to use something like StreamWriter?

Well, this usually means the native C++ stream I/O classes from the <iostream> header (for console I/O) and <fstream> for file operations. (Overview of the IOstream library that, among others, contains both of them under http://www.cplusplus.com/reference/iostream/.) Unfortunately, the .NET StreamWriter does not belong to tose.

If you are using the VC++ Express Edition, however, it's hard to make native C++ GUI apps because it lacks many of the features from the Professional version that make this task convenient. In that case it is actually easier to use C++/CLI (if you can cope with that somehow special language and the .NET framework.) The requirements you quoted don't talk about a GUI at all, however...