PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197

PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197

PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197

PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197
[RESOLVED] Abusing A Constructor-VBForums
Results 1 to 6 of 6

Thread: [RESOLVED] Abusing A Constructor

  1. #1

    Thread Starter
    Super Moderator Shaggy Hiker's Avatar
    Join Date
    Aug 2002

    Resolved [RESOLVED] Abusing A Constructor

    I have a design that is making me feel queasy, and I can't say why, so I figure I'll ask for some opinions.

    A button on a certain form will launch a process (not a Process, just a sequence of steps elsewhere in the program) which may or may not result in an exception. Either way, a second form (which I'll call Results) will be displayed. It made sense to have the program do just as I described, until I realized that in many cases, the user will have to make some difficult choices before the process can get underway, but sometimes they won't have ANY choice to make.

    So, the way that makes most sense is to use a constructor in Results. In that constructor, if the user has to make some choices, they will be shown a form to help them do so. That form won't cause any exceptions on its own, but showing a form from the constructor of a second form is decidedly weird to begin with. In any case, the next step in the constructor will be to launch the process inside some exception handling. All the controls on Results are populated as a result of what happens in the process, even if the result is an exception.

    So, it's a reasonable organization, in that the purpose of Results is to show the result of the process, and allow the user to take whatever action they see fit. Furthermore, putting that stuff in the constructor means that it can happen before I even attempt to show Results, which will work pretty well from the user experience perspective.

    What makes me queasy, is that:

    1) I'm potentially launching up to two different forms in the constructor of a different form.

    2) I'm doing something that could easily trigger an exception (easily handled, but still....) in a constructor.

    The alternative seems pretty hokey, to me, and a constructor in VB seems to be just a method like any other method, so I can't say why I shouldn't do either of the things that are bothering me, but I'd like a second opinion. The data is the responsibility of Results, so the encapsulation is right, but in the constructor???
    My usual boring signature: Nothing

  2. #2
    You don't want to know.
    Join Date
    Aug 2010

    Re: Abusing A Constructor

    Doing work in constructors is always a little stinky. I think you did good to question this. It's not a good idea to treat constructors as "just another method", but demonstrating that invoves some cases you're not in, so I'll just leave it there without explanation.

    Here's a rule of thumb. "A constructor is what I call when I have everything I need to create an object." Parameters should be the only thing it accesses, and this is a case where I think treating Law of Demeter as a law rather than a guideline is important.

    Imagine a PictureBox control that can't function unless you give it an Image first. Maybe it can take a String URL and load a picture from the internet. But if that fails, the constructor throws. That's going to manifest in the constructor of some form, and generally that's hidden in the VB framework from you. You'd have no good way to catch or respond to this exception. That's why controls in general take no constructor parameters and expect you to manipulate them via properties. This is called "The Component Pattern" in Framework Design Guidelines, and it can make life a little easier.

    So I think you're describing something like this:

    * The process begins.
    * If the user needs to make some choices, some form that collects 
        information is displayed.
    * Stuff happens, using those choices if they were made or defaults 
        if they weren't.
    * Finally, Results is displayed with the results of the operation.
    I'd like to use code that looks like this:
    Sub DoTheThing()
        Dim infoGatherer As New InformationGatherer()
        Dim info As ProcessConfiguration = infoGatherer.GetInfo()
        Dim finalResults As FinalResults = Nothing
            Dim worker As New TheThingThatDoesStuff()
            finalResults = worker.DoStuff(info)        
        Catch (ex As DoStuffException)
            finalResults = FinalResults.FromError(ex)
        End Try
        Dim results As New Results()
        results.FinalResults = finalResults
    End Sub
    I'm pretty sure you don't need a lot of details about the implmentations of the intermediate classes. Here's why I choose this design:
    • InformationGatherer.GetInfo() is just saying "I need a configuration object". It doesn't have to know if a form will be displayed. It can return default values without showing a form when no user intervention's needed. Either way, this form doesn't have to know about that form.
    • TheThingThatDoesStuff doesn't have to worry about how to fetch its information, or know that any Forms are involved. It demands its configuration information and either returns FinalResults or throws an Exception.
    • Results, as a form, only exists to display FinalResults instances. It doesn't have to know or care how they were obtained.

    This isolates most of the components from each other. The piece, as a whole, only has to coordinate gathering and displaying data. Each collaborator is only responsible for fetching one thing, which may or may not involve user intervention. That's decided at a "low" level, and never exposed to the "high" level. If one piece changes, the number of other things that change is generally easy to predict. And this algorithm might not need to change at all.

    For example, say you need to add another form to InformationGatherer. That'll add properties to ProcessConfiguration. And TheThingThatDoesStuff.DoStuff() needs to respect those new properties. But FinalResults might not change, and if it doesn't the form Results won't change either. And none of those changes require you to update this Sub.

    So try breaking each individual task into a separate class, and try to write classes that ask for what they need, rather than fetching it. That leads to writing higher level classes that fetch information, then give it to lower-level classes that produce results. Forms are generally those higher-level classes. (This is the first step towards 'Inversion of Control': letting someone else make decisions about 'how I get my data' and instead focusing on 'what I do with my data'.)

  3. #3
    Frenzied Member PlausiblyDamp's Avatar
    Join Date
    Dec 2016
    Newport, UK

    Re: Abusing A Constructor

    I always tend to be a bit wary of doing too much in a constructor, especially things that might fail and throw an exception - it just never feels right in my mind.

    When construction of an object might fail I tend to make the constructor private and provide a shared method (Create, open etc) that wraps the actual call to the constructor. (Similar to using File.Open() rather than New File() )

    In the scenario you have outlined I would lean towards a form that requests this information from a user and then this collected information is passed into the second form's constructor; although admitedly this may not be the best way in your case.

  4. #4

    Thread Starter
    Super Moderator Shaggy Hiker's Avatar
    Join Date
    Aug 2002

    Re: Abusing A Constructor

    Sitten, what you said pointed out a failing in my design that will remove part of what I said. If the user has to supply some information, they have a chance to opt not to. In that case, and that case only, the Results form does no good. Therefore, I need to move the acquisition out of the constructor. I hadn't considered that, largely because I hadn't built the acquisition forms, yet (and still don't quite know what I want to do with them, but that's a different matter).

    The process itself does seem to belong in the constructor. I can move it elsewhere, it just doesn't feel quite right to do that, either. The process is a really odd one, as it's the launch of a test of some other part of the program. The Results form is showing the result of an automated iterative test, where plugins can register tests that the user can then run in whatever sequence they choose, some of which could be impossible. Since this is a test, the test can fail and recover, or can fail catastrophically. Both are quite valuable data points, and the whole point of the Results form. So, while I'm inclined to move the acquisition of requirements out of Results, I'm inclined to leave the test in there, wrapped in a Try...Catch, because it is entirely contained in the scope and purpose of the Results form.

    Besides, I'm likely the only person to ever use the form.

    @plausiblyDamp: I totally agree, normally. In fact, in C++, any exception thrown by a constructor has to be handled in there, because the constructor has to leave the class in a known state, which an exception might not. In this case, an exception IS data, and quite valuable data, at that. In fact, an exception is almost as good as no exception. So long as it is noted, and handled, I'm fine with it in this case.
    My usual boring signature: Nothing

  5. #5
    Join Date
    Oct 2010

    Re: Abusing A Constructor

    For the logic flow you have described, I think some sort Factory pattern would be applicable.

    You mentioned that you are using a plugin mechanism to register (define?) the tests that can be run. I have roughed out a conceptual interface system to demonstrate how I might approach this while making many assumptions that are probably invalid. Obviously, much detail is missing as I have no clue what you are doing, but the overall flow avoids the pitfalls that are making you question your design.

    The ITest interface would be implemented by your plugin.

    VB.Net Code:
    1. Public Interface ITest
    2.     Function RunTest() As Boolean
    3.     Property Criteria As IList(Of ICriterion)
    4.     Property Results As String
    5. End Interface
    7. Public Interface ICriterion
    8.     Property Name As String
    9.     Property ValueType As CriterionValueType
    10. End Interface
    12. Public Class Criterion(Of T) : Implements ICriterion
    13.     Public Property Name As String Implements ICriterion.Name
    14.     Public Property ValueType As CriterionValueType Implements ICriterion.ValueType
    15.     Public Property Value As T
    16. End Class
    18. Public Enum CriterionValueType
    19.     [String]
    20.     Int32
    21.     [Double]
    22. End Enum

    Then the plugin would be something like this:
    VB.Net Code:
    1. Class Test1 : Implements ITest
    2.     Public Sub New()
    3.         ' define parameters needed to run the test
    4.         Me.Criteria = New ICriterion(0 To 1) {} ' 2 parameters need to run test
    5.         Me.Criteria(0) = New Criterion(Of String)
    6.         Me.Criteria(1) = New Criterion(Of Double)
    7.     End Sub
    9.     Public Property Criteria As IList(Of ICriterion) Implements ITest.Criteria
    10.     Public Property Results As String Implements ITest.Results
    12.     Public Function RunTest() As Boolean Implements ITest.RunTest
    13.         Return RunInternal()
    14.     End Function
    16.     Private Function RunInternal() As Boolean
    17.         ' figure it out
    18.     End Function
    19. End Class

    The factory class:
    VB.Net Code:
    1. Friend NotInheritable Class TestProcess
    2.     Private testsToRun As IList(Of ITest)
    3.     Private inputForm As Form
    4.     Private resultsForm As Form
    6.     Private Sub New()
    7.     End Sub
    9.     Public Shared Function Create(testsToRun As IList(Of ITest)) As TestProcess
    10.         Dim ret As New TestProcess
    11.         ret.testsToRun = testsToRun
    12.         ret.BuildInputForm()
    13.         ret.BuildResultsForm()
    14.     End Function
    16.     Public Sub Run()
    17.         Dim inputFormResult As DialogResult = DialogResult.OK
    18.         If inputForm IsNot Nothing Then
    19.             inputForm.ShowDialog()
    20.         End If
    21.         If inputFormResult = DialogResult.OK Then
    22.             For Each test As ITest In Me.testsToRun
    23.                 test.RunTest()
    24.                 'load results to results form
    25.             Next
    26.         End If
    27.     End Sub
    29.     Private Sub BuildInputForm()
    30.         ' logic to create an input form to get parameters for each test
    31.         ' possibly a Form with a TabControl w/ TabPage for each test
    32.         For Each test As ITest In Me.testsToRun
    33.             For Each criterion As ICriterion In test.Criteria
    35.             Next
    36.         Next
    37.     End Sub
    39.     Private Sub BuildResultsForm()
    40.         ' logic to create an Results form to show test results
    41.         ' possibly a Form with a TabControl w/ TabPage for each test
    42.     End Sub
    43. End Class

    Finally usage:
    VB.Net Code:
    1. Public Class Form1
    2.     Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    3.         Dim proc As TestProcess = TestProcess.Create(GetRegisteredTests)
    4.         proc.Run()
    5.     End Sub
    6.     Private Function GetRegisteredTests() As IList(Of ITest)
    7.     ' return IList of registered tests
    8.     End Function
    9. End Class

  6. #6

    Thread Starter
    Super Moderator Shaggy Hiker's Avatar
    Join Date
    Aug 2002

    Re: Abusing A Constructor

    That's a heckuva lot of work done. Unfortunately, it's in the wrong direction, since I left out many details. The tests aren't plugins. There are plugins, and each is a mini-program that attaches to a main structure. Many of these plugins have nothing worth testing, but some of them do. What I came up with is a mechanism such that, if I decided that a plugin had something worth testing, they could implement a test of their choosing, and had a standard mechanism to expose that to yet another plugin, such that the user could opt to perform the test, or it could be performed automatically.

    However, sleep is a good thing. This morning, I got thinking about some of the points I made earlier. For example, I mentioned getting some information from the user to perform the test. Upon further reflection, I can't come up with a single case where that would be useful. In fact, in every case where I thought it would be useful, further contemplation revealed that it was actually a bad idea. As long as I am saying, "test this item with this input", I am restricting the value of the test, because I am testing only for those cases I can anticipate. The real value of the test would be to test for cases I hadn't anticipated, and I can do that, but only without inputs.

    So, that part went out the window, and the rest....I'm not feeling queasy about. It's just feeling right. The form does something then reports the results. That's pretty much exactly what a class instance should do.
    My usual boring signature: Nothing

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts


Click Here to Expand Forum to Full Width