invoke method is not working about unicon HOT 16 CLOSED

@IanTrudel tests are very welcome, thank you for doing the work.

My pleasure! tests/unicon/tester.icn is very good but it's rather minimalist. With a bit of guidance from you guys, I can come up with a framework that offers convenience, high cohesion and extensibility.

FYI, we do have tests run as part of the CI (see Test here). We are close to changing that so that any failed test would fail the CI. We are in the process of fixing the remaining failing tests or at least isolating them.

Great work on the CI, by the way! I was surprised to see support for so many targets.

from unicon.

Okay, there are lots of interesting things going wrong here. Starting with the simple and heading towards the complex:

1. The class "Object" no longer subclasses the class "Class", so objecttests.icn needs to change all references to the Class class into references to the Object class.

2. The procedure instanceof in uni/lib/predicat.icn currently calls the istype procedure. istype appears to be broken, possibly in several ways (for one thing it calls get_type but get_type doesn't traverse the inheritance hierarchy). A workaround would be to modify the instanceof method to call lang::Type instead of istype. The correct thing may be to fix get_type but who knows what that may break?

3. In uni/lib/object.icn, the method invoke calls hasMethod, which has the line:

   return mName == genMethods()

however, genMethods now calls the builtin function methodnames to generate the names of methods. But methodnames returns the internal names of class methods, e.g. "DTest__write". I think this is wrong but, again, what breaks if it's fixed? One would think that a workaround would be to modify genmethods to map the internal names back into external ones. That's not sufficient because invoke(), assuming that's fixed, will die because the table lookup it uses (self.__m[mName]) will fail because the external name, e.g. "DTest::write" won't be found in __m - it needs to be just "write", so a 2nd workaround is needed to map the full external method name into its simple form. Note that fixing the 'wrong' methodnames function to return the simple form of method names would be sufficient unto itself (he claims).

from unicon.

This is an amazingly insightful reply. Thank you! My current workaround is to use __m[mname] but it's not without its flaws. It requires the receiver object to be passed on as the first parameter, i.e. object.__m[mnane](object), otherwise it will send it to self. In the following code example, you will notice that the output is Child::print() instead of Parent::print(), which is not a typical behaviour for OOP. It would most likely require to call o.__m[mname](self) in the run method of the Parent class.

Output:

Anonymous::print()
Child::print()

class Parent(__objects)
   method add(o)
      put(__objects, o)
   end

   method print()
      write(classname(self) || "::print()")
   end

   method run()
      local mname

      every o := !__objects do {
         cname := classname(o)

         every m := !methodnames(o) do {
            m ? {
               tab(upto(cname || "_")) & move(*cname + 1)
               mname := tab(0)
            }
            o.__m[mname](o)
         }
      }
   end

initially
   __objects := []
end

class Child : Parent()
   method print()
      self.Parent.print()
   end
end

class Anonymous()
   method print()
      write(classname(self) || "::print()")
   end
end

procedure main()
   p := Parent()
   a := Anonymous()
   c := Child()
   p.add(a)
   p.add(c)
   p.run()
end

from unicon.

I'm not sure what's causing the "Child::print()" output or even it's the "Right Thing To Do" but one suggestion for your code: in place of upto(cname||""), use find(cname||"") the upto function converts its first argument to a cset and goes up to the first appearance of any character in that cset. You're actually lucky that the class names you've got work with upto.

I assume also that this is a test program to explore behavior. Ordinarily, omitting the print method from Child would default a call to print to the Parent's print() [but still output "Child::print()", of course]. However, the run() method won't find the Parent's print method in that case. I suspect you know that and are just experimenting.

from unicon.

Ah, the reason run() doesn't find the print() method in Parent when the subclass doesn't have it is because methodnames() is only returning the 'local' methods, not any derived from ancestors. This is, he claims, another problem with the methodnames implementation. The __m list has all the methods (included inherited ones) in it but methodnames() only provides the local ones. I ;think fixing methodnames() and invoke() may be non-trivial.

from unicon.

I played around a bit with your test program and made some changes. The big ones are that the run() method:

(a) now takes as a argument the name of the method (e.g. "print") to run. This version doesn't accept any arguments to pass to that method - that should be fixed.

(b) now can find a method that exists back in the class hierarchy (it avoids using the broken methodnames)

(c) only involkes the first instance of a method from the hierarchy, starting from the 'local' and going up into superclasses

Assuming the issue stated in (a) is addressed, I think the run() method points to a viable replacement for the invoke() method.

Anyway, the amended code:

class Parent(__objects)
method add(o)
put(__objects, o)
end

method print()
write(classname(self) || "::print()")
end

method run(s) # Pass in name of method to run, should also pass in args but doesn't yet...
local mname

  every o := !__objects do {
     cname := classname(o)

     # If we find that method, then call it and stop looking for it!
     #   Note: this is prelim version that doesn't pass any args to method.
     every m := image(!o.__m) do {
        m ? {
           mname := (="procedure ", tab(0))  | next
           if mname ? (tab(-(*s+1)),="_",match(s)) then break mname(o)
        }
     }
  }

end

initially
__objects := []
end

class Child : Parent()
end

class Anonymous()
method print()
write(classname(self) || "::print()")
end
end

procedure main()
p := Parent()
a := Anonymous()
c := Child()
p.add(a)
p.add(c)
p.run("print")
end

from unicon.

from unicon.

Here's a potential replacement for the invoke() method in uni/lib/object.icn. It avoids the problem of mapping between internal/external/simple method names. I've only tested it a little, so caveat emptor!

method invoke(s, A[])
local mname

  every image(!self.__m) ? {
     mname := (="procedure ", tab(0)) | next
     if mname ? (tab(-(*s+1),="_",match(s)) then
           suspend mname ! ([self]|||A) | fail
     }

end

from unicon.

Oops. Typo. The change would be to uni/lib/object.icn, not uni/lib/class.icn. Sorry!

You can edit a previous post by pressing the three dots at the top right corner of post.

from unicon.

RE: upto() vs find()

Thank you for the tip!

RE: methodnames() only listing the class methods (no parent).

I believe this is acceptable considering that Unicon implements multiple inheritance. One could end up with several methods with the same name. The way one would list the parent's methods would perhaps be something like methodnames(classname(self.Parent)).

RE: alternative to __m invocation.

I tried something like that but couldn't get it to work. Calling mname(o) and the likes is certainly less convoluted.

RE: self in Child vs self in Parent

Smalltalk has defined the behaviour of self to be bound by the class it is used in. In Child, self should be the current instance of Child, and, in Parent, self should be the current instance of Parent. For some reason, Unicon seems to flatten the concept of self where there is no class hierarchy. This is the point I was illustrating in the code example.

RE: classobject.icn, errorsystem.icn, utf8.icn

@brucerennie this is how I figured out how to properly use __m. I would still be stuck without your work.

from unicon.

I have been writing a unit test framework based on Kent Beck's original SUnit. The idea is to fully embrace Unicon OOP and also better integrate with your CI, where failing tests would result in failed build — then you don't get to wait years before someone shows up with issues like those in objecttests.

This is work in progress. I am working on assertions and then a TestReporter. The TestReporter will be a class on its own because someone may someday want to integrate it to the Unicon IDE, they can write their own TestReporter accordingly.

Hope this clarifies a few things.

link ximage

import lang

invocable all

class TestCase : Object()
   method setupClass()
      write(classname(self), "::setupClass()")
   end

   method teardownClass()
      write(classname(self), "::teardownClass()")
   end

   method setup()
      write(classname(self), "::setup()")
   end

   method teardown()
      write(classname(self), "::teardown()")
   end

   method testNoTest()
      write("shouldn't be here")
   end

   method assertEqual(n, m)
      local retval

      retval := success

      write("assertEqual")

      # are n, m strings? integers? reals?
      # case type(n) of {
      #  "string": { }
      #  "integer": { }
      # }
      if n ~= m then {
         write("Assertion failed: Expected ", m, ", but got ", n)
         retval := failure
      }

      return retval
   end

   method print()
      write("print has been called.")
   end
end

class TestSuite(__tests, __testReporter)
   method add(testCase)
      cname := classname(testCase)
      if (\cname & (not (cname == "TestCase"))) then {
         if \testCase.__m["Type"] then {
            every ctype := testCase.Type() do {
               if ctype == "TestCase" then {
                  put(__tests, testCase)
               }
            }
         }
      }
   end

   method run()
      local passed, failed, methodName

      passed := 0
      failed := 0

      every test := !__tests do {
         cname := classname(test)

         test.setupClass()
         every m := !methodnames(test) do {
            m ? {
               tab(find(cname || "_")) & move(*cname + 1)
               methodName := tab(0)
            }
            if methodName ? match("test") then {
               write("\t" || classname(test) || "::" || methodName)
               test.setup()
               test.__m[methodName](test)
               test.teardown()
            }
         }
         test.teardownClass()
      }

      self.output()
   end

   method output()
      write(ximage(__tests))
   end

   method print()
      write("print from TestSuite.")
   end

initially
   __tests := []
   /__testReporter := "ReporterClass"
end

class NoClass()
end

class NormalTest : TestCase()
   method this_is_not_a_test()
      write("shouldn't happen.")
   end

   method testSomething()
      write("100101010010")
   end

   method test_me_one_time()
      assertEqual(2, 5)
   end

   method testOneMore()
      #assertEqual("a", "b")
   end

   method testNormal()
      write("o_O")
   end
end

class AnotherTest : TestCase()
   method setupClass()
      write("AnotherTest class setup.")
   end

   method setup()
      write("Another test, another day. Setup.")
   end

   method testAnother()
      write("Running another test.")
   end

   method teardown()
      write("Another test, another day. Teardown.")
   end

   method teardownClass()
      write("AnotherTest class teardown.")
   end
end

procedure main()
   ts := TestSuite()
   tc := TestCase()
   nc := NoClass()
   nt := NormalTest()
   at := AnotherTest()
   ts.add(&null)
   ts.add("hello")
   ts.add(tc)
   ts.add("o_O")
   ts.add(nc)
   ts.add(nt)
   ts.add(at)
   ts.run()
end

from unicon.

@IanTrudel tests are very welcome, thank you for doing the work.

FYI, we do have tests run as part of the CI (see Test here). We are close to changing that so that any failed test would fail the CI. We are in the process of fixing the remaining failing tests or at least isolating them.

from unicon.

Here's a potential replacement for the invoke() method in uni/lib/object.icn. It avoids the problem of mapping between internal/external/simple method names. I've only tested it a little, so caveat emptor!

method invoke(s, A[]) local mname

  every image(!self.__m) ? {
     mname := (="procedure ", tab(0)) | next
     if mname ? (tab(-(*s+1),="_",match(s)) then
           suspend mname ! ([self]|||A) | fail
     }

end

@StephenWampler Your solution turned out pretty well. It's clean and straightforward.

      every test := !__tests do {
         cname := classname(test)

         test.setupClass()
         every image(!test.__m) ? {
            mname := (="procedure ", tab(0)) | next
            if mname ? (tab((*cname+1)), ="_", match("test")) then {
               test.setup()
               mname(test)
               test.teardown()
            }
         }
         test.teardownClass()
      }

from unicon.

@StephenWampler, wanna go after some of the issues you identified? We should attempt to fix those. If they break any existing applications we can discuss.

from unicon.

Invoke should be fixed with issue #441. Also, despite multiple inheritance, only one of any duplicated method is ever visible to the subclass. There's a fix in #441 so all of the visible methods available to a subclass are produced by genMethods(). objecttests.icn now produces the expected output. Note that the function (i.e. "builtin") methodnames is still broken. My C is so far in the past the I'm absolutely the wrong person to try and fix it. I know. I looked. It's pretty much greek to me...

from unicon.

I'm going to close this issue since invoke() now works as expected. I'm also going to open a new issue on the fact that methodnames() doesn't generate inherited methods visible to the subclass.

from unicon.