Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix namespace of Swt: SwtShoes::* -> Shoes::Swt::* #6

Closed
wasnotrice opened this issue May 25, 2012 · 21 comments
Closed

Fix namespace of Swt: SwtShoes::* -> Shoes::Swt::* #6

wasnotrice opened this issue May 25, 2012 · 21 comments
Labels

Comments

@wasnotrice
Copy link
Member

As discussed in shoes/brown_shoes#3

@ashbb
Copy link
Member

ashbb commented May 29, 2012

Hi all, look at this again to recall the code structure.
Okay now, let's take one step further. :)

I've tried to make a minimum Shoes 4 branch. This is a personal sandbox for experiment.
Download the sandbox and execute $ bin/swt-shoooes samples/test1.rb

I got the following result.

D:\tmp\shoes4>bin\swt-shoooes samples\test1.rb

D:\tmp\shoes4>call jruby --1.9 -e "$:<< 'lib'; require 'shoes'; Shoes.backend =
:SWT; require 'samples\test1.rb' "
#<Shoes::App:0xeabd2f>
Shoes::Check
#<Java::OrgEclipseSwtWidgets::Button:0x779959>

D:\tmp\shoes4>
  • The files depended on SWT are just lib/shoes/swt.rb and lib/shoes/swt/* only. They are isolated.
  • The Swt stuff are kept in the Shoes namespace. (Refer to lib/shoes/swt/swt.rb)

I think that the code structure in this sandbox is reasonable. What do you think about it?

@pjfitzgibbons
Copy link
Member

In your sandbox,
Shoes::Swt needs to remove :
include_package 'org.eclipse.swt'
include_package 'org.eclipse.swt.layout'
include_package 'org.eclipse.swt.widgets'

The 'swt' gem handles these, and they are available inside the 'swt' gem at ::Swt::SWT

@alindeman
Copy link
Contributor

Agreed. I started on this a bit too, and think it's good to move stuff to lib/shoes/swt. And to get rid of non-useful code either along the way or afterward.

@wasnotrice
Copy link
Member Author

@ashbb this is a great practice. I notice one problem, but I'm not sure what is the best way to solve it.

I think the output should be

<Shoes::App:0xeabd2f>
Shoes::Check
<Shoes::Swt::Check:0x779959>

I started trying to make that happen in your code, and realized that it requires 2 levels of delegation:

Shoes::Check -> Shoes::Swt::Check -> Java::OrgEclipseSwtWidgets::Button

I tried this idea out in ashbb#1

I don't know if this is a good idea or not. It seems too complicated. But I don't think it works well to

  1. Add methods to Swt classes
  2. Subclass Swt classes

So I think we might need that other layer.

@pjfitzgibbons
Copy link
Member

Yep, I agree w Eric. It isn't pretty. The framework acts as a shim
between Shoes and the low level GUI ( in this case swt)
It is for this reason that I am wary of patterns in this design. There
is essentially an inversion of control issue from two directions ( or
two delegations. Or two decorators or...)
This is why I did not code _before/_after. The shim led me towards a
custom callback design

Peter Fitzgibbons
Sent from my iPhone

On May 29, 2012, at 5:34 PM, Eric Watson
reply@reply.github.com
wrote:

@ashbb this is a great practice. I notice one problem, but I'm not sure what is the best way to solve it.

I think the output should be

<Shoes::App:0xeabd2f>
Shoes::Check
<Shoes::Swt::Check:0x779959>

I started trying to make that happen in your code, and realized that it requires 2 levels of delegation:

Shoes::Check -> Shoes::Swt::Check -> Java::OrgEclipseSwtWidgets::Button

I tried this idea out in ashbb#1

I don't know if this is a good idea or not. It seems too complicated. But I don't think it works well to

  1. Add methods to Swt classes
  2. Subclass Swt classes

So I think we might need that other layer.


Reply to this email directly or view it on GitHub:
#6 (comment)

@wasnotrice
Copy link
Member Author

So this popped into my mind as I was washing dishes. I have noticed there have been NO DRAWINGS to speak of yet in this Shoes 4 discussion. Tsk, tsk. Next time, maybe I will even use color :)

Shoes 3 is like a tricycle

You push the pedal, the wheel goes around. Pedals are tightly coupled to the wheel. You may not use a different wheel.

Shoes 4 is like a bicycle

You push the pedal, the wheel goes around. The pedals are loosely coupled to the wheel. You may use different wheels.

@pjfitzgibbons
Copy link
Member

Well done! I like 'em as b&w line art.

So...
Shoes On Wheels!
:)
Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Tue, May 29, 2012 at 10:50 PM, Eric Watson <
reply@reply.github.com

wrote:

So this popped into my mind as I was washing dishes. I have noticed there
have been NO DRAWINGS to speak of yet in this Shoes 4 discussion. Tsk,
tsk. Next time, maybe I will even use color :)

Shoes 3 is like a tricycle

You push the pedal, the wheel goes around. Pedals are tightly coupled to
the wheel. You may not use a different wheel.

Shoes 4 is like a bicycle

You push the pedal, the wheel goes around. The pedals are loosely coupled
to the wheel. You may use different wheels.


Reply to this email directly or view it on GitHub:
#6 (comment)

@alindeman
Copy link
Contributor

Very awesome @wasnotrice

@alindeman
Copy link
Contributor

These could/should be added to some documentation or the README :)

@wasnotrice
Copy link
Member Author

Well, thanks for the compliments! :)

The question is: do the pictures help to clarify/explain/rationalize the proposed architecture? And also, how can you illustrate the current architecture?

@ashbb
Copy link
Member

ashbb commented May 30, 2012

@wasnotrice said in the above post

I think the output should be

#<Shoes::App:0xeabd2f>
Shoes::Check
#<Shoes::Swt::Check:0x779959>

Umm,.. sorry, I don't agree with you. I still think the output should be

#<Shoes::App:0xeabd2f>
Shoes::Check
#<Java::OrgEclipseSwtWidgets::Button:0x779959>

@pjfitzgibbons said in the above post

The framework acts as a shim between Shoes and the low level GUI ( in this case swt)

I agree. But even so, I don't want to complicate the code.

I revised the sandbox. Please review the code structure again.

@wasnotrice
Copy link
Member Author

@ashbb this is why I think we need Shoes::Swt::Check. I could be totally wrong, but this is why.

In your implementation, we don't control the interface to @real, because we don't "own" that class. If we are making a "pluggable" architecture, where you might use Swt and I might use Cairo, then from the Shoes perspective, @real must have the same interface.

In terms of the bicycle diagram above, I think your code is permanently coupling the chain to the rear sprocket, so that we cannot simply replace the wheel with a different one and re-use the same chain, front chainring, and pedals.

As I read your code, when we want to check the Shoes::Check, we will have something like

module Shoes
  class Check
    def check
      @real.set_selection true
    end
  end
end

That will work for Swt. But how will it work for Cairo, or Swing, or Qt?

I'm going to rename a little here and say we need:

module Shoes
  class Check
    def check
      @gui.check # where @gui.class # => Shoes::Swt::Check
    end
  end
end

module Shoes
  module Swt
    class Check
      def check
        @real.set_selected true # where @real.class # => Java::OrgEclipseSwtWidgets::Button
      end
    end
  end
end

Then we can easily add other backend implementations in the same way. I think this is more work now, but will be less work later.

Does that make sense?

@ashbb
Copy link
Member

ashbb commented May 30, 2012

@wasnotrice Thank you for the explanation. Now I understood your story. :)

we can easily add other backend implementations in the same way.

I agree. But your story is for developers, not for users.

My story is for users. So, users (i.e. Shoes app programmers) can write the following snippet within their code (i.e. their Shoes app)!

module Shoes
  class Check
    def check
      @real.set_selection true
    end
  end
end

But okay. If our goal is making a "pluggable" architecture, I agree your story. :)

@wasnotrice
Copy link
Member Author

@ashbb WAIT WAIT WAIT :)

It's true that my story is for developers. That's because I think the Shoes DSL _should not change_. A Shoes app programmer should write

checkbox = check "Don't change Shoes DSL"
checkbox.check = true

The code above should _not_ be used by Shoes app programmers ;)

@pjfitzgibbons
Copy link
Member

Um... correct me if I'm wrong, Shoesers...
This code snippet is not something a Shoes end-user is anticipated to use.
No?
Steve?

module Shoes
 class Check
   def check
     @real.set_selection true
   end
 end
end

Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Wed, May 30, 2012 at 10:23 AM, ashbb <
reply@reply.github.com

wrote:

@wasnotrice Thank you for the explanation. Now I understood your story. :)

we can easily add other backend implementations in the same way.

I agree. But your story is for developers, not for users.

My story is for users. So, users (i.e. Shoes app programmers) can write
the following snippet within their code (i.e. their Shoes app)!

module Shoes
 class Check
   def check
     @real.set_selection true
   end
 end
end

But okay. If our goal is making a "pluggable" architecture, I agree your
story.


Reply to this email directly or view it on GitHub:
#6 (comment)

@ashbb
Copy link
Member

ashbb commented May 31, 2012

@wasnotrice, @pjfitzgibbons, @alindeman and folks,

I still think the output should be

Shoes.app do
  p self      #=> #<Shoes::App:0xeabd2f>
  c = check
  p c.class   #=> Shoes::Check
  p c.real    #=> #<Java::OrgEclipseSwtWidgets::Button:0x779959>
end

But, I understand that we need Shoes::Swt::Check to make Shoes 4 into a bicycle. ;-)

So, I improved my sandbox again. Now, we can write like this:

# lib/shoes/check.rb
module Shoes
  class Check
    def check
      gui_check
    end
  end
end

# lib/shoes/swt/check.rb
module Shoes
  module Swt
    module Check
      def gui_check
        @real.set_selected true
      end
    end
  end
end

Can we say we've reached a basic consensus? Or need further discussion?

@ashbb ashbb closed this as completed May 31, 2012
@ashbb ashbb reopened this May 31, 2012
@ashbb
Copy link
Member

ashbb commented May 31, 2012

Oops, mistakenly pushed Close & comment button. xx-P

@pjfitzgibbons
Copy link
Member

Now I feel you have achieved the separation I expected.

For the Shoes-user-developer, the backend is "hidden".
For the Shoes-backend-developer, the real-gui is available.

I like it.
What do we need to do next?

Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Thu, May 31, 2012 at 6:46 AM, ashbb <
reply@reply.github.com

wrote:

@wasnotrice, @pjfitzgibbons, @alindeman and folks,

I still think the output should be

Shoes.app do
 p self      #=> #<Shoes::App:0xeabd2f>
 c = check
 p c.class   #=> Shoes::Check
 p c.real    #=> #<Java::OrgEclipseSwtWidgets::Button:0x779959>
end

But, I understand that we need Shoes::Swt::Check to make Shoes 4 into a
bicycle. ;-)

So, I improved my sandbox again. Now, we can write like this:

# lib/shoes/check.rb
module Shoes
 class Check
   def check
      gui_check
   end
 end
end

# lib/shoes/swt/check.rb
module Shoes
 module Swt
   module Check
     def gui_check
       @real.set_selected true
     end
   end
 end
end

Can we say we've reached a basic consensus? Or need further discussion?


Reply to this email directly or view it on GitHub:
#6 (comment)

@wasnotrice
Copy link
Member Author

@ashbb @pjfitzgibbons I have further discussion :P

Full code is in my sandbox pull request ashbb#2

This version uses classes rather than modules for the backend objects like Shoes::Swt::Check. This makes testing much easier. Also, I have included your real so that both of your examples work as expected :)

Here's the check code (compare to #6 (comment))

# lib/shoes/check.rb
module Shoes
  class Check
    def initialize app
      @gui = Shoes.backend::Check.new(app)
    end
    attr_reader :gui

    def real
      @gui.real
    end
  end
end

# lib/shoes/swt/check.rb
class Shoes::Swt::Check
  def initialize(app)
    @real = Swt::Widgets::Button.new app.gui.shell, Swt::SWT::CHECK
  end
  attr_accessor :real
end

What do you think?

@pjfitzgibbons
Copy link
Member

I'm IN.

Keep going

Shoes On!

Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Thu, May 31, 2012 at 9:49 AM, Eric Watson <
reply@reply.github.com

wrote:

@ashbb @pjfitzgibbons I have further discussion :P

Full code is in my sandbox pull request
ashbb#2

This version uses classes rather than modules for the backend objects like
Shoes::Swt::Check. This makes testing much easier. Also, I have
included your real so that both of your examples work as expected :)

Here's the check code (compare to
#6 (comment))

# lib/shoes/check.rb
module Shoes
 class Check
    def initialize app
     @gui = Shoes.backend::Check.new(app)
   end
   attr_reader :gui

   def real
     @gui.real
    end
 end
end

# lib/shoes/swt/check.rb
class Shoes::Swt::Check
 def initialize(app)
   @real = Swt::Widgets::Button.new app.gui.shell, Swt::SWT::CHECK
 end
 attr_accessor :real
end

What do you think?


Reply to this email directly or view it on GitHub:
#6 (comment)

ashbb added a commit to ashbb/shoes4 that referenced this issue Jun 1, 2012
@ashbb
Copy link
Member

ashbb commented Jun 1, 2012

#written by @wasnotrice

This version uses classes rather than modules for the backend objects like
Shoes::Swt::Check. This makes testing much easier. Also, I have included your
real so that both of your examples work as expected :)

Great! I like your solution. Now I think we've reached a basic consensus. :)
I merged your pull request. This is the final version:

Try out the following three examples. They works as I expect.

$ bin/swt-shoooes samples/test1.rb
$ bin/swt-shoooes samples/test2.rb
$ bin/swing-shoooes samples/teest1.rb

#written by @pjfitzgibbons

What do we need to do next?

Close this issue and start to revise shoes4/spec and shoes4/lib in accordance with our new code structure to make Shoes 4 into a bicycle. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants