[Solved] Opinions on two code layout alternatives?
-
I have an application that reads a directory's files, displays them in a grid, and offers a toolbar with buttons that invoke functions that apply to all the files in the grid. In the following code, reduced to its essentials, when the user clicks the Trim button in the toolbar,
toolTrim_Clickexecutes, opening the form that collects the options for the Trim operation, and then invoking the method that trims the pages from the list of files. That method then invokes for each file the method that trims the pages from that particular file. Theasyncpart is just to keep the window draggable.//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
await Task.Run(() => filesList.TrimPages(form.trimFirst, form.trimLast, form.ifBlank));
}
form.Dispose();
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
FileEntry thisFile = list[entry];
thisFile.Trim(trimFirst, trimLast, ifBlank);
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}What would be the pros and cons of refactoring the above code like this? The difference is that we move opening the options form to the class that manages the list of files instead of keeping it in the main form class.
//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
await Task.Run(() => filesList.TrimPages());
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
PDFEntry thisFile = list[entry];
}
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}It seems to me that the class that manages the list of files should be the class that collects the various operation options, but I don't know which is the best practice, or what the implications are of doing it the second way.
-
I have an application that reads a directory's files, displays them in a grid, and offers a toolbar with buttons that invoke functions that apply to all the files in the grid. In the following code, reduced to its essentials, when the user clicks the Trim button in the toolbar,
toolTrim_Clickexecutes, opening the form that collects the options for the Trim operation, and then invoking the method that trims the pages from the list of files. That method then invokes for each file the method that trims the pages from that particular file. Theasyncpart is just to keep the window draggable.//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
await Task.Run(() => filesList.TrimPages(form.trimFirst, form.trimLast, form.ifBlank));
}
form.Dispose();
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
FileEntry thisFile = list[entry];
thisFile.Trim(trimFirst, trimLast, ifBlank);
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}What would be the pros and cons of refactoring the above code like this? The difference is that we move opening the options form to the class that manages the list of files instead of keeping it in the main form class.
//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
await Task.Run(() => filesList.TrimPages());
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
PDFEntry thisFile = list[entry];
}
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}It seems to me that the class that manages the list of files should be the class that collects the various operation options, but I don't know which is the best practice, or what the implications are of doing it the second way.
I see no reason to use a modal dialog. You could use a regular form and encapsulate ALL the related logic in the form. Alternatively, create a static class with a single entry point that wraps the logic and dialog. Component-based design.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
-
I see no reason to use a modal dialog. You could use a regular form and encapsulate ALL the related logic in the form. Alternatively, create a static class with a single entry point that wraps the logic and dialog. Component-based design.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
Well, I'm using modal dialogs for the various forms that collect user input because, while any of those forms is open, the user should not be able to focus on any other form. It's like Word's Font dialog. I suppose I could put all related code in each form. Then the Rename Files form would have its own code that renames files, and the Trim Pages code would have its own code that trims pages from the files. But I understood from my Google "research" that forms should just be data input and validation devices. I thought it was better practice to have a class that deals with the collection (files, invoices, parts, whatever) that contains the logic for managing the collection, and separately a class for the individual item within the collect that would contain the logic for operating on the item.
-
Well, I'm using modal dialogs for the various forms that collect user input because, while any of those forms is open, the user should not be able to focus on any other form. It's like Word's Font dialog. I suppose I could put all related code in each form. Then the Rename Files form would have its own code that renames files, and the Trim Pages code would have its own code that trims pages from the files. But I understood from my Google "research" that forms should just be data input and validation devices. I thought it was better practice to have a class that deals with the collection (files, invoices, parts, whatever) that contains the logic for managing the collection, and separately a class for the individual item within the collect that would contain the logic for operating on the item.
A "form" (i.e. Window / view) is an interface for the user to your app; it is not limited to "data input and validation". Parts of that interface include the screen, keyboard, mouse, joystick, "touch", etc. And the app you described was not a "data entry application"; it was a utility for manipulating files; which may or may not need to be modal.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
-
A "form" (i.e. Window / view) is an interface for the user to your app; it is not limited to "data input and validation". Parts of that interface include the screen, keyboard, mouse, joystick, "touch", etc. And the app you described was not a "data entry application"; it was a utility for manipulating files; which may or may not need to be modal.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
Thanks for the input. You had suggested putting all the code in the form, but that violates the principle of separation of concerns. I know the application I described is not a "data entry application." I didn't describe it as such. The question was really about code organization, not about form modality or the definition of user interface. Please let me know if you have any answer to the question posed as is. I confess I have not read the Analects, but I love and know by heart its first line. The Master said: To study and at due times practice what one has studied, is this not a pleasure?
-
I have an application that reads a directory's files, displays them in a grid, and offers a toolbar with buttons that invoke functions that apply to all the files in the grid. In the following code, reduced to its essentials, when the user clicks the Trim button in the toolbar,
toolTrim_Clickexecutes, opening the form that collects the options for the Trim operation, and then invoking the method that trims the pages from the list of files. That method then invokes for each file the method that trims the pages from that particular file. Theasyncpart is just to keep the window draggable.//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
await Task.Run(() => filesList.TrimPages(form.trimFirst, form.trimLast, form.ifBlank));
}
form.Dispose();
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
FileEntry thisFile = list[entry];
thisFile.Trim(trimFirst, trimLast, ifBlank);
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}What would be the pros and cons of refactoring the above code like this? The difference is that we move opening the options form to the class that manages the list of files instead of keeping it in the main form class.
//Main Form:
private async void toolTrim_Click(object sender, EventArgs e)
{
await Task.Run(() => filesList.TrimPages());
}//filesList Class:
public void TrimPages(bool trimFirst, bool trimLast, bool ifBlank)
{
var form = new TrimOptions();
DialogResult result = form.ShowDialog();
if (result == DialogResult.OK)
{
for (int entry = 0; entry < TotalPDFs; entry++)
{
PDFEntry thisFile = list[entry];
}
}
}//thisFile Class:
public void Trim(bool front, bool back, bool ifBlank)
{
// code
}It seems to me that the class that manages the list of files should be the class that collects the various operation options, but I don't know which is the best practice, or what the implications are of doing it the second way.
Keep the GUI classes separate. I prefer your original code over the proposed change. What if you want to build a console version of this app where you pass a wild card and an operation? Keeping the GUI separate would allow this to work without hassle. 2021*.pdf -trim first last blank
-
Keep the GUI classes separate. I prefer your original code over the proposed change. What if you want to build a console version of this app where you pass a wild card and an operation? Keeping the GUI separate would allow this to work without hassle. 2021*.pdf -trim first last blank