One of the causes of your problems is that everyone is changing your StartForm
. Apart from that this spaghetti makes it difficult to understand, it certainly doesn't help to make your classes reusable and maintainable if your Startform
changes.
It seems to me, that AppJobs
is meant to decide what the form should look like (for instance it decides that the StartForm
should change height), while FormJobs
performs the calculations needed to change this height. StartForm
apparently is just allowing to let everyone make changes to him.
A better design would be that StartForm
would not ask AppJobs
to change its size, and to ask the operator whether a folder should be generated. Instead if ought to ask appJobs
for advise: "Which height do you think I should have". after that it could ask FormJobs: "Please adjust my height according to this specification"
FormJobs
should trust StartForm
that it has gathered the correct information about how a StartForm
ought to look like. FormJobs
should not ask AppJobs
for any information: "Hey AppJobs, StartForm asked me to change its appearance to certain specifications, but I'm not certain whether StartForm has done its job correctly. Please tell me if these specifications are correct, and give me some missing information")
The correct division into tasks would be:
- AppJobs specifies the format of any
StartForm
according to its internal state (a.o. AppRoot, and existence of certain folders)
StartForm
is the one who displays all items. He decides who to ask for specifications, and what to do with the returned specifications. He is also the only one who communicates with operators
FormJobs
is a class that apparently knows all elements from a StartForm
. If you will only have one type of StartForm
, then Appjobs
should be part of the Startform
class. If you think there might be several different Startform
classes, all with the same elements that ought to be manipulated similarly, shouldn't all these StartForm
classes be derived from a FormJobs
class?
Anyway, redesign without everyone causing to manipulate StartForm
Apparently there are a limited number of StartForm
layouts depending on AppRoot, defaultDevice etc. You seem to be missing some "else" after your if, so this list might not be accurate. Still you will get the idea:
enum StartFormLayouts
{
DefaultDevice0,
AppRoot0,
Other,
}
// class that specifies the layout of any startform
class AppJobs
{
private bool IsAppRoot0
{
get{return Appjobs.AppRoot == null || Appjobs.AppRoot == "0";}
}
private bool IsDefaultDevice0
{
get{return Appjobs.DefaultDevice == null || Appjobs.DefaultDevice == "0";}
}
public StartFormLayoug GetPreferredLayout()
{
if (this.IsAppRoot0)
{
if (this.IsDefaultDevice)
{
return StartFormLayout.DefaultDevice0;
}
else
return StartFormLayout.AppRoot0;
}
else
{
return StartFormLayout.Other;
}
}
public bool ShouldAskDirectoryCreation()
{
return (!this.IsAppRoot0 && !Directory.Exists(AppRoot));
}
}
Note that this class doesn't need StartForm, nor AppJobs. It could work with any class that wants to know if it should ask for DirectoryCreation. Since it also does not speak any language, even a Chinese StartForm
could use it. After all, the StartForm
is the only one who knows what language it speaks and what to do if a certain layout is requested.
Furthermore, did you notice that I used a double ||
to use a Boolean OR instead of a bitwise or?
And I use statements like if (a)
instead of if(a=true)
a C# Boolean is a real Boolean, in contradiction to Booleans in C and C++.
The class of all kinds of forms that should be able to be layout according to the requested layout contains the functions similar to your
It depends a bit of whether you decide to let it be a base class of StartForm
or StartForm
itself. If you want it to handle every form class that has the required controls, consider of using an interface:
public Interface IStartForm
{
public int Height {get; set;}
public Label LabelNoRoot {get;}
public Label LabelNoDevice {get; }
public Button BtnTickets {get;}
...
This way you can set the size of any form that has these labels and buttons, even if they have different names than those strings you use.
But again: if you ever only want to size StartForm
, then this should be a function in StartForm
.
public SetHeight(StartFormLayout layout, IStartForm startForm)
{
switch (layout)
{
case StartFormLayout.DefaultDevice0:
if (startForm.MenuStrip.Visible)
{
startForm.Height = ...;
startForm.LabelNoRoot.Location = ...
// etc
}
else
{
...
Noticed that because of this separation of concerns the AppJobs and FormJobs don't have to know each other. AppJobs and FormJobs also don't really have to know what 'StartForm` is, only that it has the labels and buttons etc that it needs to change.
class StartForm : Form, IStartForm
{
public Label LabelNoRoot {get{return this.label1; } }
...
private void FrmStart_Load(object sender, EventArgs e)
{
AppJobs layoutdesigner = new AppJobs(...);
StartFormLayout layoutdesigner = layouter.GetPreferredLayout();
FormJobs layouter = new FormJobjs();
layouter.SetHeight(this)
}
Noticed that my form didn't have a label named "LabelNoRoot", but a Label1 that should function as a LabelNoRoot. Also: because I used types instead of string, You can be certain that I can't handle a label as if it was a button. I can't by accident try to press the label. Something that could easily been done when you were using strings to identify the items you want to layout.