Re: Why is there a "new" and "run" in every module?

Moritz Lenz <[email protected]> Mon, 29 Feb 2016 16:51:12 +0100
Newsgroups gmane.comp.otrs.devel
Message-ID <[email protected]>
This is an S/MIME signed message

--===============1218886780==
Content-Type: multipart/signed; protocol="application/x-pkcs7-signature";
	micalg="sha1"; boundary="----696ED377AD3C6319FE408F81EC48A9ED"

This is an S/MIME signed message

------696ED377AD3C6319FE408F81EC48A9ED
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Niels,

I'm not an OTRS core developer, so please take my reply with the 
necessary grain of salt.

On 02/29/2016 04:21 PM, Niels Dimmers wrote:
> Hi,
>
> In an attempt to fix bug 8104 I am restructuring a lot of code in the
> module AgentTicketZoom and the related toolbar packages (located in
> Kernel/Output/HTML/ToolBar).

Don't do that, if that's avoidable. It's much easier to get a small fix 
merged, and it's also much easier to get a refactoring merged that 
doesn't change behavior. So it's a very good idea to separate the two 
concerns.

And from skimming the bug description, it doesn't sound like a big 
restructuring is actually necessary to fix the bug.

> I found that there's a "new" and "run" sub
> in the module and in the display packages as well. Does anyone know why
> this structure has been chosen?

I'd speculate that the original idea was to create such objects only 
once, and reuse them multiple requests.

That doesn't work these days, but what it does is providing a uniform 
interface to most classes in OTRS. Which is probably worth the 
boilerplate. (Though I do plan to reduce the boilerplate a bit by 
inheriting from a new class that simply provides a default constructor).

> Isn't it far easier to use (get) and
> (set) methods to get and set data?

Many methods don't easily map to the get/set paradigma. For example many 
modules in Kernel::Modules::* offer both a read-only view and some write 
access; neither a "get" nor a "set" method would describe their purpose 
very well.

> I think it should be easier to let the model
> (Kernel/Output/HTML/ToolBar) generate and/or supply the template,
> instead of doing this partly in the contoller (AgentTicketZoom) and the
> template. I think the way it's structured now requires a lot of extra
> code and a lot of processing power whilst the same could be dealt with
> easier and cheaper. Do you have any suggestions on this? Since it is *a
> lot* of work I thought about mailing you first before applying such a
> broad restructure of the code.

Well, you could allow a Kernel::Output::HTML::ToolBar::*-Module to 
return an already rendered HTML string, and ignore all the other 
returned data if that's present. That would be a backwards-compatible 
but flexible refactoring. Another option would be to allow the module to 
return a template file name (or specify that in the sysconfig directly), 
and use the INCLUDE mechanism in the templates to render it.

Cheers,
Moritz

------696ED377AD3C6319FE408F81EC48A9ED
Content-Type: application/x-pkcs7-signature; name="smime.p7s"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="smime.p7s"

MIIY5AYJKoZIhvcNAQcCoIIY1TCCGNECAQExCzAJBgUrDgMCGgUAMAsGCSqGSIb3
DQEHAaCCFPkwggUaMIIEAqADAgECAhBtGeqnGU9qMyLmIjJ6qnHeMA0GCSqGSIb3
DQEBBQUAMIGuMQswCQYDVQQGEwJVUzELMAkGA1UECBMCVVQxFzAVBgNVBAcTDlNh
bHQgTGFrZSBDaXR5MR4wHAYDVQQKExVUaGUgVVNFUlRSVVNUIE5ldHdvcmsxITAf
BgNVBAsTGGh0dHA6Ly93d3cudXNlcnRydXN0LmNvbTE2MDQGA1UEAxMtVVROLVVT
RVJGaXJzdC1DbGllbnQgQXV0aGVudGljYXRpb24gYW5kIEVtYWlsMB4XDTExMDQy
ODAwMDAwMFoXDTIwMDUzMDEwNDgzOFowgZMxCzAJBgNVBAYTAkdCMRswGQYDVQQI
ExJHcmVhdGVyIE1hbmNoZXN0ZXIxEDAOBgNVBAcTB1NhbGZvcmQxGjAYBgNVBAoT
EUNPTU9ETyBDQSBMaW1pdGVkMTkwNwYDVQQDEzBDT01PRE8gQ2xpZW50IEF1dGhl
bnRpY2F0aW9uIGFuZCBTZWN1cmUgRW1haWwgQ0EwggEiMA0GCSqGSIb3DQEBAQUA
A4IBDwAwggEKAoIBAQCShIRbS1eY1F4vi6ThQMijU1hfZmXxMk73nzJ9VdB4TFW3
QpTg+SdxB8XGaaS5MsTxQBqQzCdWYn8XtXFpruUgG+TLY15gyqJB9mrho/+43x9I
bWVDjCouK2M4d9+xF6zC2oIC1tQyatRnbyATj1w1+uVUgK/YcQodNwoCUFNslR2p
EBS0mZVZEjH/CaLSTNxS297iQAFbSGjdxUq04O0kHzqvcV8H46y/FDuwJXFoPfQP
1hdYRhWBPGiLi4MPbXohV+Y0sNsyfuNK4aVScmQmkU6lkg//4LFg/RpvaFGZY40a
i6XMQpubfSJj06mg/M6ekN9EGfRcWzW6FvOnm//BAgMBAAGjggFLMIIBRzAfBgNV
HSMEGDAWgBSJgmd9xJ0mcABLtFBIfN49rgRufTAdBgNVHQ4EFgQUehNOAHRbxnhj
ZCfBL+KgW7x5xXswDgYDVR0PAQH/BAQDAgEGMBIGA1UdEwEB/wQIMAYBAf8CAQAw
EQYDVR0gBAowCDAGBgRVHSAAMFgGA1UdHwRRME8wTaBLoEmGR2h0dHA6Ly9jcmwu
dXNlcnRydXN0LmNvbS9VVE4tVVNFUkZpcnN0LUNsaWVudEF1dGhlbnRpY2F0aW9u
YW5kRW1haWwuY3JsMHQGCCsGAQUFBwEBBGgwZjA9BggrBgEFBQcwAoYxaHR0cDov
L2NydC51c2VydHJ1c3QuY29tL1VUTkFkZFRydXN0Q2xpZW50X0NBLmNydDAlBggr
BgEFBQcwAYYZaHR0cDovL29jc3AudXNlcnRydXN0LmNvbTANBgkqhkiG9w0BAQUF
AAOCAQEAhda+eFdVbTN/RFL+QtUGqAEDgIr7DbL9Sr/2r0FJ9RtaxdKtG3NuPukm
fOZMmMEwKN/L+0I8oSU+CnXW0D05hmbRoZu1TZtvryhsHa/l6nRaqNqxwPF1ei+e
upN5yv7ikR5WdLL4jdPgQ3Ib7Y/9YDkgR/uLrzplSDyYPaUlv73vYOBJ5RbI6z9D
g/Dg7g3B080zX5vQvWBqszv++tTJOjwf7Zv/m0kzvkIpOYPuM2kugp1FTahp2oAb
Hj3SGl18R5mlmwhtEpmG1l1XBxunML5LSUS4kH7K0Xk467Qz+qA6XSZYnmFVGLQh
1ZnV4ENAQjC+6qXnlNKw/vN1+X9u5zCCBN0wggPFoAMCAQICEHGS++YZX6xNEoV0
cTSiGKcwDQYJKoZIhvcNAQEFBQAwezELMAkGA1UEBhMCR0IxGzAZBgNVBAgMEkdy
ZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UEBwwHU2FsZm9yZDEaMBgGA1UECgwRQ29t
b2RvIENBIExpbWl0ZWQxITAfBgNVBAMMGEFBQSBDZXJ0aWZpY2F0ZSBTZXJ2aWNl
czAeFw0wNDAxMDEwMDAwMDBaFw0yODEyMzEyMzU5NTlaMIGuMQswCQYDVQQGEwJV
UzELMAkGA1UECBMCVVQxFzAVBgNVBAcTDlNhbHQgTGFrZSBDaXR5MR4wHAYDVQQK
ExVUaGUgVVNFUlRSVVNUIE5ldHdvcmsxITAfBgNVBAsTGGh0dHA6Ly93d3cudXNl
cnRydXN0LmNvbTE2MDQGA1UEAxMtVVROLVVTRVJGaXJzdC1DbGllbnQgQXV0aGVu
dGljYXRpb24gYW5kIEVtYWlsMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
AQEAsjmFpPJ9q0E7YkY3rs3BYHW8OWX5ShpHornMSMxqmNVNNRm5pELlzkniii8e
fNIxB8dOtINknS4p1aJkxIW9hVE1eaROaJB7HHqkkqgX8pgV8pPMyaQylbsMTzC9
mKALi+VuG6JG+ni8om+rWV6lL8/K2m2qL+usobNqqrcuZzWLeeEeaYji5kbNoKXq
vgvOdjp6Dpvq/NonWz1zHyLmSGHGTPNpsaguG7bUMSAsvIKKjqQOpdeJQ/wWWq8d
cdcRWdq6hw2v+vPhwvCkxWeM1tZUOt4KpLoDd7NlyP0e03RiqhjKaJMeoYV+9Udl
y/hNVyh00jT/MLbu9mIwFIws6wIDAQABo4IBJzCCASMwHwYDVR0jBBgwFoAUoBEK
Iz6W8Qfs4q8p74Klf9AwpLQwHQYDVR0OBBYEFImCZ33EnSZwAEu0UEh83j2uBG59
MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdJQQWMBQGCCsG
AQUFBwMCBggrBgEFBQcDBDARBgNVHSAECjAIMAYGBFUdIAAwewYDVR0fBHQwcjA4
oDagNIYyaHR0cDovL2NybC5jb21vZG9jYS5jb20vQUFBQ2VydGlmaWNhdGVTZXJ2
aWNlcy5jcmwwNqA0oDKGMGh0dHA6Ly9jcmwuY29tb2RvLm5ldC9BQUFDZXJ0aWZp
Y2F0ZVNlcnZpY2VzLmNybDARBglghkgBhvhCAQEEBAMCAQYwDQYJKoZIhvcNAQEF
BQADggEBAJ2Vyzy4fqUJxB6/C8LHdo45PJTGEKpPDMngq4RdiVTgZTvzbRx8Nywl
VF+WIfw3hJGdFdwUT4HPVB1rbEVgxy35l1FM+WbKPKCCjKbI8OLp1Er57D9Wyd12
jMOCAU9sAPMeGmF0BEcDqcZAV5G8ZSLFJ2dPV9tkWtmNH7qGL/QGrpxp7en0zykX
2OBKnxogL5dMUbtGB8SKN04g4wkxaMeexIud6H4RvDJoEJYRmETYKlFgTYjrdDrf
QwYyyDlWjDoRUtNBpEMD9O3vMyfbOeAUTibJ2PU54om4k123KSZB6rObroP8d3XK
6Mq1/uJlSmM+RMTQw16Hc6mYHK9/FX8wggQyMIIDGqADAgECAgEBMA0GCSqGSIb3
DQEBBQUAMHsxCzAJBgNVBAYTAkdCMRswGQYDVQQIDBJHcmVhdGVyIE1hbmNoZXN0
ZXIxEDAOBgNVBAcMB1NhbGZvcmQxGjAYBgNVBAoMEUNvbW9kbyBDQSBMaW1pdGVk
MSEwHwYDVQQDDBhBQUEgQ2VydGlmaWNhdGUgU2VydmljZXMwHhcNMDQwMTAxMDAw
MDAwWhcNMjgxMjMxMjM1OTU5WjB7MQswCQYDVQQGEwJHQjEbMBkGA1UECAwSR3Jl
YXRlciBNYW5jaGVzdGVyMRAwDgYDVQQHDAdTYWxmb3JkMRowGAYDVQQKDBFDb21v
ZG8gQ0EgTGltaXRlZDEhMB8GA1UEAwwYQUFBIENlcnRpZmljYXRlIFNlcnZpY2Vz
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvkCd9G7h6naHHE1FRI6+
RsiDBp3BKv4YH47kAvrzq11QihYxC5oG0MVwIs1JLVRjzLZuaEYLU+rLTCTAvHJO
6vEVrvRUmhIKw3qyM2Di2olV8yJY897cz++DhqKMlE+faPKYkEaEJ8d2v+PMNSyL
XgdkZYLASLCokflhn3YgUKiRx2a163hiA1bwihoT6jGjHqCZ/Tj29icyWG8H9Wu4
+xQrr7eqzNZjX3OM2gWZqDioyxd4NlGs6Z70eDqNzw/ZQuKYDKsvnw4B3u+fmUnx
Ld+sdE0bmLVHxeUp0fmQGMdinL6DxyZ7Poolx8DdneY1aBAgnY/Y3tLDhJwNXugv
yQIDAQABo4HAMIG9MB0GA1UdDgQWBBSgEQojPpbxB+zirynvgqV/0DCktDAOBgNV
HQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zB7BgNVHR8EdDByMDigNqA0hjJo
dHRwOi8vY3JsLmNvbW9kb2NhLmNvbS9BQUFDZXJ0aWZpY2F0ZVNlcnZpY2VzLmNy
bDA2oDSgMoYwaHR0cDovL2NybC5jb21vZG8ubmV0L0FBQUNlcnRpZmljYXRlU2Vy
dmljZXMuY3JsMA0GCSqGSIb3DQEBBQUAA4IBAQAIVvwC8Jvo/6T61nvGRIDOT8TF
9gBYzKa2vBRJaAR26ObuXewCD2DWjVAYTyZOAePmsKXuv7x0VEG//fwSuMdPWvSJ
YAV/YLcFSvP28cK/xLl0hrYtfWvM0vNG3S/G4GrDwzQDLH2W3VrCDqcKmcEFi6sM
L/NcOs9sN1UJh95TQGxY7/y2q2VuBPYb3DzgWhXGntnxWUgwIWUDbOzpIXPsmwOh
4DetoBUYj/q6As6nLKkQEyzU5QgmqyKXYPiQXnTUoppTvfKpaOCibsLXbLGjD56/
62jnVvKu8uMrODoJgbVrhde+Le0/GreyY+L1YiyC1GoAQVDxOYOflek2lphuMIIG
wDCCBaigAwIBAgIQS4frQ5t4dGHt0koUSNGOjjANBgkqhkiG9w0BAQsFADCBmzEL
MAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UE
BxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxQTA/BgNVBAMT
OENPTU9ETyBTSEEtMjU2IENsaWVudCBBdXRoZW50aWNhdGlvbiBhbmQgU2VjdXJl
IEVtYWlsIENBMB4XDTE1MDYxMDAwMDAwMFoXDTE2MDYwOTIzNTk1OVowgbsxEjAQ
BgNVBAcMCU7DvHJuYmVyZzESMBAGA1UECAwJTsO8cm5iZXJnMQswCQYDVQQGEwJE
RTEdMBsGA1UEAwwUbW9yaXR6LmxlbnpAbm9yaXMuZGUxJTAjBgNVBAsTHChFLU1h
aWwgR2F0ZXdheSBDZXJ0aWZpY2F0ZSkxGTAXBgNVBAoTEG5vcmlzIG5ldHdvcmsg
QUcxIzAhBgkqhkiG9w0BCQEWFG1vcml0ei5sZW56QG5vcmlzLmRlMIICIjANBgkq
hkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAznBuDEwu6TxhnxovrB5Q1K5rYZMKSpMz
88RTjem9M1q2unQBCOSNa46YWL36xy6L4xfLr+Z/EGja1Pm4+FWQNOEBRDLPPAfX
s/UJt8beK3JGTOCe8KPhYLITzCah6njd3QdM1pwY1eE0dqEIohfzWXmx1qNNdseq
8AvyHniYTSHtmh7YK1y0H+wMPPCG9XxIDpT/RDt2hydvV9HEE0VgL5SyISznFFzm
mNa62zTfe2dMTukOilo4MOLKmgtE2LraAlZVAxb7OE8TLGmfRhesvWAGxP2hyVyj
YC717EL+d6OUsO7OYsdB+y+Mbgxd3ymO6W62UE+F9wNjZyNz9lpmCfZdIyUUJIL4
+Kt6AMsaT1HJ+BIkgSCd66Mx6dQQ8dEPx792ilMlWdazWX2UIdlAdEgrHTCKiggG
glWb6fq1KXPh1HZc3tZ3dLDx/vnd4GAz1tgk05v9//QlTrhpw0tT2T4ddsbhFHGl
RnUHl/7weVgb5nxjs+AtJfX24bM/cJFKfuMgIoA22Pj+9Bbu/iDrn7qjz9XVg1I0
Vou1Ic+ahIQ15mfXHFVBEl6gY7JEj07qffH/NUo/WUu7jzEavkmT3T/5/XV9J6N2
u0Un7DFW58E7XaOdmjOVnRiQ949aoOv0iW93GVoI7/wWxybm2SETYwtGNF8QPujy
vYACJ0zajH8CAwEAAaOCAdwwggHYMB8GA1UdIwQYMBaAFJJha4LhoqCqT+xn8cKj
97SAAMHsMB0GA1UdDgQWBBQLzc0qwSavkTsZB/5Kx7ZtHT0RyTAOBgNVHQ8BAf8E
BAMCBaAwDAYDVR0TAQH/BAIwADAdBgNVHSUEFjAUBggrBgEFBQcDBAYIKwYBBQUH
AwIwRgYDVR0gBD8wPTA7BgwrBgEEAbIxAQIBAwUwKzApBggrBgEFBQcCARYdaHR0
cHM6Ly9zZWN1cmUuY29tb2RvLm5ldC9DUFMwXQYDVR0fBFYwVDBSoFCgToZMaHR0
cDovL2NybC5jb21vZG9jYS5jb20vQ09NT0RPU0hBMjU2Q2xpZW50QXV0aGVudGlj
YXRpb25hbmRTZWN1cmVFbWFpbENBLmNybDCBkAYIKwYBBQUHAQEEgYMwgYAwWAYI
KwYBBQUHMAKGTGh0dHA6Ly9jcnQuY29tb2RvY2EuY29tL0NPTU9ET1NIQTI1NkNs
aWVudEF1dGhlbnRpY2F0aW9uYW5kU2VjdXJlRW1haWxDQS5jcnQwJAYIKwYBBQUH
MAGGGGh0dHA6Ly9vY3NwLmNvbW9kb2NhLmNvbTAfBgNVHREEGDAWgRRtb3JpdHou
bGVuekBub3Jpcy5kZTANBgkqhkiG9w0BAQsFAAOCAQEAPYhmGlBfOrfvI+mNLPGG
QT+6AyouOT6B2WCPnuetEaeSPHdXd+/adL19w7c8GzQpHJmqiYEuiqrTb99jyaJh
yidVRrn0WKsev83DamswwUgtcRPgfPqoabfBqIeffMVepOjM/ADfJ2gpkjefMAta
ckSltogQ5Y5I/LWhzpmNduqAo/HHwDuBG4CGdRG3x+BBMmlnz+KaFB7VrdIUedXH
6rq8aLoY0vQbE+qqR5dAroyNuyn4LuaZnRkECHE1xSfqJ1xCDvw22Q2KxlKVG16t
yZEDpdOJlKHIwOF2hQDHZFloEOJhps1D9Sq9vj+syi9wxhmKSQZtFUaStsmi4Ls5
oDGCA7MwggOvAgEBMIGwMIGbMQswCQYDVQQGEwJHQjEbMBkGA1UECBMSR3JlYXRl
ciBNYW5jaGVzdGVyMRAwDgYDVQQHEwdTYWxmb3JkMRowGAYDVQQKExFDT01PRE8g
Q0EgTGltaXRlZDFBMD8GA1UEAxM4Q09NT0RPIFNIQS0yNTYgQ2xpZW50IEF1dGhl
bnRpY2F0aW9uIGFuZCBTZWN1cmUgRW1haWwgQ0ECEEuH60ObeHRh7dJKFEjRjo4w
CQYFKw4DAhoFAKCB2DAYBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwGCSqGSIb3
DQEJBTEPFw0xNjAyMjkxNTUwNDNaMCMGCSqGSIb3DQEJBDEWBBSUzqfo++o4z8fB
VT8RyumYwzg5hzB5BgkqhkiG9w0BCQ8xbDBqMAsGCWCGSAFlAwQBKjALBglghkgB
ZQMEARYwCwYJYIZIAWUDBAECMAoGCCqGSIb3DQMHMA4GCCqGSIb3DQMCAgIAgDAN
BggqhkiG9w0DAgIBQDAHBgUrDgMCBzANBggqhkiG9w0DAgIBKDANBgkqhkiG9w0B
AQEFAASCAgDImvrLOOZsE7fHN4ORBxdKDKmLQGdVHxgWWlC2gWCDN4lM7pas3Q6R
seNGXQlvl0KJkz5bYihg5XKbqbWY6Qv9EAF75YeZfXbG4iSo91IYsGYO+Ve9aR+8
Lm+GqTjlm9BC6mXwWnXT4V2o/D/f6o6llcZDpkzrMKI01Y8UoO1o7xGgNIchZ1/3
Kz8hfuAhUS6t/6YBWEi7MaqKJG1qXZIFKhkFHaH6l25pFT1+hHgTFzQ5+z17PKGj
0/XmF/dARLcZArFB45ROC7UFVTPQeor5x3xhYYSMIJofR5V87OfJYaTRCZdEVu3F
9eWk610GBRwly1jDdI5ihbMr1ZUKFfRMXEZl4TewQ1I5OANJdcc24BxaSqD5bhf2
bFAJeqvEoPko3hrSJWs7ISL+HMh0QCf2aRUsIzFfqGI37m9WZUOMJz6d3QOIWhTc
WiF7tjS8cNsGnpBih21zhPQFcucetefEEHJ7FixlINTwZhBpVOg5KrHYJFIPtJkb
4dpwdIdGKNe3Vq2y7ihNka1Agcgv90pSo29euNWmzWJKFJkz+uVT+CD8pLFdFbSF
aHypvyYXM54X87ddhfpw7IE3WmuINa6v8P7K16x86fEJiRpyktoknYXhmOnbh7ug
weG25L8qLWKRLgNFpUpBfDpEDfhTLG6IpXk4fn5OEJOqcGkX0r2mzg==

------696ED377AD3C6319FE408F81EC48A9ED--


--===============1218886780==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
OTRS mailing list: dev - Webpage: http://otrs.org/
Archive: http://lists.otrs.org/pipermail/dev
To unsubscribe: http://lists.otrs.org/cgi-bin/listinfo/dev
--===============1218886780==--