Re: [Koha-devel] [Koha::Object] Related records and calling methods from templates

[David Cook]

Agreed with what Kyle has said here.

 

Especially with the part about considering templates as our View and that View 
logic is OK. I don’t think we should be handling View logic in the controller 
script if it can be avoided.

 

David Cook

Systems Librarian

Prosentient Systems

72/330 Wattle St

Ultimo, NSW 2007

Australia

 

Office: 02 9212 0899

Direct: 02 8005 0595

 

From: koha-devel-boun...@lists.koha-community.org 
[mailto:koha-devel-boun...@lists.koha-community.org] On Behalf Of Kyle Hall
Sent: Wednesday, 21 September 2016 2:37 AM
To: Jonathan Druart <jonathan.dru...@bugs.koha-community.org>
Cc: koha-devel@lists.koha-community.org
Subject: Re: [Koha-devel] [Koha::Object] Related records and calling methods 
from templates

 


What do you suggest? It's much easier to always get and pass a full
object than sometimes a smaller one without knowing exactly what it
represents (which methods you can call on it).
I don't think it will hurt perfs too much.
Of course there will certainly be some rooms for improvements here, it
should not be a rule written in stone.

 

I agree. Passing the full object to the template will actually affect the 
performance the least. What we pass is a ref. If we start munging the data 
before we give it to the template, we are just adding needless overdead.

 


> We start passing these objects to the templates now too, making it possible
> to call methods from the template. It is possible, but do we really want
> that? More code ends up in the template, maybe harder to find since you now
> use borrower.method instead of borrower->method etc. Secondly, reading
> borrower.something might not immediately tell you that something is a method
> instead of a column.

Yes, I don't think it's a good thing. We should avoid calling methods
in template, only accessors. If the method is trivial, I'd say it
could be ok.

 

I think it's important to consider the template is our View, and logic is fine 
in the template as long as it's View logic. Formatting a date for example. It 
shouldn't matter whether we call and object method or a plugin to handle that 
view logic. In fact, I'd put money on calling an object method being faster and 
more efficient.

 


> In other cases we have added a sort of wrapper like Branches.GetName from
> the Branches plugin.

That was to help us, and that will continue to help us, because the fk
don't always exist.
If should not hurt perf neither.

 

We can work around the lack for fk constrains within our objects as well!

 


> I am interested to know if we want to enforce this wrapper approach for
> instance, or just continue on the current road.

Where and why? :)

Usually I don't think TT plugins should be used for such things. In an
ideal world, we would manipulate objects in templates and
library.branchname would display the library name (in a ideal world it
would be library.name...).
Or patron.library.branchname, but we should not need a plugin for
that: Branches.GetName( patron.branchcode )

 

I agree with this completely. I think it makes for a much more natural language 
and a cleaner implementation.

 

Kyle

___
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Re: [Koha-devel] [Koha::Object] Related records and calling methods from templates

[Kyle Hall]

>
>
> What do you suggest? It's much easier to always get and pass a full
> object than sometimes a smaller one without knowing exactly what it
> represents (which methods you can call on it).
> I don't think it will hurt perfs too much.
> Of course there will certainly be some rooms for improvements here, it
> should not be a rule written in stone.
>

I agree. Passing the full object to the template will actually affect the
performance the least. What we pass is a ref. If we start munging the data
before we give it to the template, we are just adding needless overdead.


>
> > We start passing these objects to the templates now too, making it
> possible
> > to call methods from the template. It is possible, but do we really want
> > that? More code ends up in the template, maybe harder to find since you
> now
> > use borrower.method instead of borrower->method etc. Secondly, reading
> > borrower.something might not immediately tell you that something is a
> method
> > instead of a column.
>
> Yes, I don't think it's a good thing. We should avoid calling methods
> in template, only accessors. If the method is trivial, I'd say it
> could be ok.
>

I think it's important to consider the template is our View, and logic is
fine in the template as long as it's View logic. Formatting a date for
example. It shouldn't matter whether we call and object method or a plugin
to handle that view logic. In fact, I'd put money on calling an object
method being faster and more efficient.


>
> > In other cases we have added a sort of wrapper like Branches.GetName from
> > the Branches plugin.
>
> That was to help us, and that will continue to help us, because the fk
> don't always exist.
> If should not hurt perf neither.
>

We can work around the lack for fk constrains within our objects as well!


>
> > I am interested to know if we want to enforce this wrapper approach for
> > instance, or just continue on the current road.
>
> Where and why? :)
>
> Usually I don't think TT plugins should be used for such things. In an
> ideal world, we would manipulate objects in templates and
> library.branchname would display the library name (in a ideal world it
> would be library.name...).
> Or patron.library.branchname, but we should not need a plugin for
> that: Branches.GetName( patron.branchcode )
>

I agree with this completely. I think it makes for a much more natural
language and a cleaner implementation.

Kyle
___
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Re: [Koha-devel] [Koha::Object] Related records and calling methods from templates

[Jonathan Druart]

2016-09-15 13:03 GMT+02:00 Marcel de Rooy :
> Hi devs,
>

Hi :)

> I am seeing more Koha objects that include methods for related records like
> Koha::Patron, sub image referring to Koha::Patron::Images.
>
> As a second example, on bug 14610 (article requests), a biblio, item, branch
> and borrower method is added to Koha::ArticleRequest.

These methods should highlight the relationships (fk or missing fk at
DB level) between objects. They make sense to me as long as we want to
manipulate Koha::Objects.

Note that branch should be replaced with library ;)

> Obviously, they all follow the same pattern.
>
> Could we generalize that, and keep our code cleaner?
>
>
>
> And do we always need the full Koha object? For use in a template we may
> need only a few columns.

What do you suggest? It's much easier to always get and pass a full
object than sometimes a smaller one without knowing exactly what it
represents (which methods you can call on it).
I don't think it will hurt perfs too much.
Of course there will certainly be some rooms for improvements here, it
should not be a rule written in stone.

> We start passing these objects to the templates now too, making it possible
> to call methods from the template. It is possible, but do we really want
> that? More code ends up in the template, maybe harder to find since you now
> use borrower.method instead of borrower->method etc. Secondly, reading
> borrower.something might not immediately tell you that something is a method
> instead of a column.

Yes, I don't think it's a good thing. We should avoid calling methods
in template, only accessors. If the method is trivial, I'd say it
could be ok.

> In other cases we have added a sort of wrapper like Branches.GetName from
> the Branches plugin.

That was to help us, and that will continue to help us, because the fk
don't always exist.
If should not hurt perf neither.

> I am interested to know if we want to enforce this wrapper approach for
> instance, or just continue on the current road.

Where and why? :)

Usually I don't think TT plugins should be used for such things. In an
ideal world, we would manipulate objects in templates and
library.branchname would display the library name (in a ideal world it
would be library.name...).
Or patron.library.branchname, but we should not need a plugin for
that: Branches.GetName( patron.branchcode )

Cheers

> Thx,
>
>
>
> Marcel
>
>
>
>
> ___
> Koha-devel mailing list
> Koha-devel@lists.koha-community.org
> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> website : http://www.koha-community.org/
> git : http://git.koha-community.org/
> bugs : http://bugs.koha-community.org/
___
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Re: [Koha-devel] [Koha::Object] Related records and calling methods from templates

[David Cook]

I'm not entirely sure what you mean in the first paragraph, so I don't have
much to add there.

 

Personally, I like having access to objects in the templates. In terms of
CRUD, it seems to make writing interfaces much easier and cleaner. I suppose
you could say that more code ends up in the template, but you could also say
that more of the presentation ends up in the template as well I think. I
suppose that's a debateable point. As for borrower.something vs
borrower->something, I don't know if it matters if it looks like a method or
a column. It would be quick enough to find out if it mattered. 

 

I'm not sure what you mean by the wrapper approach exactly, but personally I
think we could do with fewer wrappers and just try to keep things as simple
as possible.  Re-reading your example, do you mean that you'd pass a branch
code to the template and then use Branches.GetName from the Branches plugin
to replace that with the branch name? In theory, it would be nice if you
could do item.branchcode.branchname (where branchcode is a foreign key and
using the branchname method causes the Item object to fetch the
corresponding Branch object), which is something that I recall Jonathan
doing perhaps in some of the templates? Of course, I think using the plugin
approach, you might be able to add more multilingual support for other
things. For instance. item.itype would retrieve "BOOK".
Item.itype.item_description would be "Book", but if you used
ItemTypes.GetName (or something like that), you could possibly choose a
translated template instead of just the value from the database.

 

Since most of my work is in English, I'll defer to others regarding
translations in templates.

David Cook

Systems Librarian

Prosentient Systems

72/330 Wattle St

Ultimo, NSW 2007

Australia

 

Office: 02 9212 0899

Direct: 02 8005 0595

 

From: koha-devel-boun...@lists.koha-community.org
[mailto:koha-devel-boun...@lists.koha-community.org] On Behalf Of Marcel de
Rooy
Sent: Thursday, 15 September 2016 9:03 PM
To: koha-devel@lists.koha-community.org
Subject: [Koha-devel] [Koha::Object] Related records and calling methods
from templates

 

Hi devs,

 

I am seeing more Koha objects that include methods for related records like
Koha::Patron, sub image referring to Koha::Patron::Images.

As a second example, on bug 14610 (article requests), a biblio, item, branch
and borrower method is added to Koha::ArticleRequest.

Obviously, they all follow the same pattern.

Could we generalize that, and keep our code cleaner?

 

And do we always need the full Koha object? For use in a template we may
need only a few columns.

We start passing these objects to the templates now too, making it possible
to call methods from the template. It is possible, but do we really want
that? More code ends up in the template, maybe harder to find since you now
use borrower.method instead of borrower->method etc. Secondly, reading
borrower.something might not immediately tell you that something is a method
instead of a column.

In other cases we have added a sort of wrapper like Branches.GetName from
the Branches plugin.

I am interested to know if we want to enforce this wrapper approach for
instance, or just continue on the current road.

 

Thx,

 

Marcel

 

___
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-devel] [Koha::Object] Related records and calling methods from templates

[Marcel de Rooy]

Hi devs,

I am seeing more Koha objects that include methods for related records like 
Koha::Patron, sub image referring to Koha::Patron::Images.
As a second example, on bug 14610 (article requests), a biblio, item, branch 
and borrower method is added to Koha::ArticleRequest.
Obviously, they all follow the same pattern.
Could we generalize that, and keep our code cleaner?

And do we always need the full Koha object? For use in a template we may need 
only a few columns.
We start passing these objects to the templates now too, making it possible to 
call methods from the template. It is possible, but do we really want that? 
More code ends up in the template, maybe harder to find since you now use 
borrower.method instead of borrower->method etc. Secondly, reading 
borrower.something might not immediately tell you that something is a method 
instead of a column.
In other cases we have added a sort of wrapper like Branches.GetName from the 
Branches plugin.
I am interested to know if we want to enforce this wrapper approach for 
instance, or just continue on the current road.

Thx,

Marcel

___
Koha-devel mailing list
Koha-devel@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/