Insult my code!

Hi there,

I'm in the process of trying to wrap my head around MVC, and as part of
that, I'm attempting to implement a super-tiny MVC framework.

I've created some mockups of how the framework might be used based
around a very simple 'bank', but I'm trying to get some feedback before
I go and implement it, to make sure I'm actually on the right track.

Any thoughts would be much appreciated!

Model - http://www.pastebin.cz/23595
Controller - http://www.pastebin.cz/23597
View - http://www.pastebin.cz/23598
Template - http://www.pastebin.cz/23599

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

2009/10/7 Eric Bauman :

> Any thoughts would be much appreciated!

One observation. "Model" isn't a synonym for "Database Table" - models
can be anything that encapsulates business logic. Requiring all your
models to inherit from Model is probably a bad idea.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

RE: Insult my code!

U2VlbXMgb2suDQoNCkp1c3QgYSB0aG91Z2h0Og0KDQogICAgICAgIFlvdXIg bW9kZWwgc2VlbXMg
dG8gYmUgY29kZWQganVzdCBmb3IgcmV0cmVpdmluZyBkYXRhLiBJTU8geW91 IHNob3VsZCBjb2Rl
IGl0IGZvciBhbGwgcG9zc2libGUgYWN0aW9ucyhpbnNlcnQsdXBkYXRlLGRl bGV0ZSxzZWxlY3Qp
LiBBbmQgYWxzbyBpdCBzaG91bGQgcnVuIHdpdGhvdXQgYW55IGRhdGFiYXNl IGNhbGxzKHlvdSBt
YXkgY3JlYXRlIGEgbmV3IGJhbmsgdXNlciBpbiBhIHBhZ2UgYW5kIHVzZSBp dCB0aGFuIHRocm93
IGl0IGF3YXksIHNvIHlvdSB3b250IGhhdmUgdG8gbmVlZCBhIGRhdGFiYXNl IHRvIHNhdmUgaXQp
Lg0KDQogICAgICAgIFlvdXIgbW9kZWwgbWF5IGJlIG1vcmUgdXNlYWJsZSB3 aGVuIGl0IGNhbiBo
b2xkcyB0aGUgZGF0YSBpdHNlbGYod2l0aG91dCByZXRyZWl2aW5nIGl0IGZy b20gZGIgZmlyc3Qp
LCBhZnRlciB0aGF0IGEgc2F2ZSgpIG1ldGhvZCBtYXkgaW5zZXJ0cy91cGRh dGVzIGl0IHRvIGRi
Lg0KDQoNCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEVy aWMgQmF1bWFuIFtt
YWlsdG86YmF1bWFuZUBsaXZlam91cm5hbC5ka10NClNlbnQ6IFdlZG5lc2Rh eSwgT2N0b2JlciAw
NywgMjAwOSA5OjM1IEFNDQpUbzogcGhwLWdlbmVyYWxAbGlzdHMucGhwLm5l dA0KU3ViamVjdDog
W1BIUF0gSW5zdWx0IG15IGNvZGUhDQoNCkhpIHRoZXJlLA0KDQpJJ20gaW4g dGhlIHByb2Nlc3Mg
b2YgdHJ5aW5nIHRvIHdyYXAgbXkgaGVhZCBhcm91bmQgTVZDLCBhbmQgYXMg cGFydCBvZg0KdGhh
dCwgSSdtIGF0dGVtcHRpbmcgdG8gaW1wbGVtZW50IGEgc3VwZXItdGlueSBN VkMgZnJhbWV3b3Jr
Lg0KDQpJJ3ZlIGNyZWF0ZWQgc29tZSBtb2NrdXBzIG9mIGhvdyB0aGUgZnJh bWV3b3JrIG1pZ2h0
IGJlIHVzZWQgYmFzZWQNCmFyb3VuZCBhIHZlcnkgc2ltcGxlICdiYW5rJywg YnV0IEknbSB0cnlp
bmcgdG8gZ2V0IHNvbWUgZmVlZGJhY2sgYmVmb3JlDQpJIGdvIGFuZCBpbXBs ZW1lbnQgaXQsIHRv
IG1ha2Ugc3VyZSBJJ20gYWN0dWFsbHkgb24gdGhlIHJpZ2h0IHRyYWNrLg0K DQpBbnkgdGhvdWdo
dHMgd291bGQgYmUgbXVjaCBhcHByZWNpYXRlZCENCg0KTW9kZWwgLSBodHRw Oi8vd3d3LnBhc3Rl
YmluLmN6LzIzNTk1DQpDb250cm9sbGVyIC0gaHR0cDovL3d3dy5wYXN0ZWJp bi5jei8yMzU5Nw0K
VmlldyAtIGh0dHA6Ly93d3cucGFzdGViaW4uY3ovMjM1OTgNClRlbXBsYXRl IC0gaHR0cDovL3d3
dy5wYXN0ZWJpbi5jei8yMzU5OQ0KDQotLQ0KUEhQIEdlbmVyYWwgTWFpbGlu ZyBMaXN0IChodHRw
Oi8vd3d3LnBocC5uZXQvKQ0KVG8gdW5zdWJzY3JpYmUsIHZpc2l0OiBodHRw Oi8vd3d3LnBocC5u
ZXQvdW5zdWIucGhwDQoNCg0KQnUgbWVzYWogdmUgZWtsZXJpLCBtZXNhamRh IGfDtm5kZXJpbGRp
xJ9pIGJlbGlydGlsZW4ga2nFn2kva2nFn2lsZXJlIMO2emVsZGlyIHZlIGdp emxpZGlyLiBTaXpl
IHlhbmzEscWfbMSxa2xhIHVsYcWfbcSxxZ9zYSBsw7x0ZmVuIGfDtm5kZXJl biBraXNpeWkgYmls
Z2lsZW5kaXJpbml6IHZlIG1lc2FqxLEgc2lzdGVtaW5pemRlbiBzaWxpbml6 LiBNZXNhaiB2ZSBl
a2xlcmluaW4gacOnZXJpxJ9pIGlsZSBpbGdpbGkgb2xhcmFrIMWfaXJrZXRp bWl6aW4gaGVyaGFu
Z2kgYmlyIGh1a3VraSBzb3J1bWx1bHXEn3UgYnVsdW5tYW1ha3RhZMSxci4g xZ5pcmtldGltaXog
bWVzYWrEsW4gdmUgYmlsZ2lsZXJpbmluIHNpemUgZGXEn2nFn2lrbGnEn2Ug dcSfcmF5YXJhayB2
ZXlhIGdlw6cgdWxhxZ9tYXPEsW5kYW4sIGLDvHTDvG5sw7zEn8O8bsO8biB2 ZSBnaXpsaWxpxJ9p
bmluIGtvcnVuYW1hbWFzxLFuZGFuLCB2aXLDvHMgacOnZXJtZXNpbmRlbiB2 ZSBiaWxnaXNheWFy
IHNpc3RlbWluaXplIHZlcmViaWxlY2XEn2kgaGVyaGFuZ2kgYmlyIHphcmFy ZGFuIHNvcnVtbHUg
dHV0dWxhbWF6Lg0KDQpUaGlzIG1lc3NhZ2UgYW5kIGF0dGFjaG1lbnRzIGFy ZSBjb25maWRlbnRp
YWwgYW5kIGludGVuZGVkIGZvciB0aGUgaW5kaXZpZHVhbChzKSBzdGF0ZWQg aW4gdGhpcyBtZXNz
YWdlLiBJZiB5b3UgcmVjZWl2ZWQgdGhpcyBtZXNzYWdlIGluIGVycm9yLCBw bGVhc2UgaW1tZWRp
YXRlbHkgbm90aWZ5IHRoZSBzZW5kZXIgYW5kIGRlbGV0ZSBpdCBmcm9tIHlv dXIgc3lzdGVtLiBP
dXIgY29tcGFueSBoYXMgbm8gbGVnYWwgcmVzcG9uc2liaWxpdHkgZm9yIHRo ZSBjb250ZW50cyBv
ZiB0aGUgbWVzc2FnZSBhbmQgaXRzIGF0dGFjaG1lbnRzLiBPdXIgY29tcGFu eSBzaGFsbCBoYXZl
IG5vIGxpYWJpbGl0eSBmb3IgYW55IGNoYW5nZXMgb3IgbGF0ZSByZWNlaXZp bmcsIGxvc3Mgb2Yg
aW50ZWdyaXR5IGFuZCBjb25maWRlbnRpYWxpdHksIHZpcnVzZXMgYW5kIGFu eSBkYW1hZ2VzIGNh
dXNlZCBpbiBhbnl3YXkgdG8geW91ciBjb21wdXRlciBzeXN0ZW0uDQo=

Re: Insult my code!

On 7/10/2009 7:36 PM, Mert Oztekin wrote:
> Seems ok.
>
> Just a thought:
>
> Your model seems to be coded just for retreiving data. IMO you should code it for all possible actions(insert,update,delete,select). And also it should run without any database calls(you may create a new bank user in a page and use it than throw it away, so you wont have to need a database to save it).
>
> Your model may be more useable when it can holds the data itself(without retreiving it from db first), after that a save() method may inserts/updates it to db.
>
>
>
> -----Original Message-----
> From: Eric Bauman [mailto:baumane@livejournal.dk]
> Sent: Wednesday, October 07, 2009 9:35 AM
> To: php-general@lists.php.net
> Subject: [PHP] Insult my code!
>
> Hi there,
>
> I'm in the process of trying to wrap my head around MVC, and as part of
> that, I'm attempting to implement a super-tiny MVC framework.
>
> I've created some mockups of how the framework might be used based
> around a very simple 'bank', but I'm trying to get some feedback before
> I go and implement it, to make sure I'm actually on the right track.
>
> Any thoughts would be much appreciated!
>
> Model - http://www.pastebin.cz/23595
> Controller - http://www.pastebin.cz/23597
> View - http://www.pastebin.cz/23598
> Template - http://www.pastebin.cz/23599
>
> --
> PHP General Mailing List (http://www.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
> Bu mesaj ve ekleri, mesajda gönderildiği belirtilen kişi/kişilere özeldir ve gizlidir. Size yanlışlıkla ulaşmışsa lütfen gönderen kisiyi bilgilendiriniz ve mesajı sisteminizden siliniz. Mesaj ve eklerinin içeriği ile ilgili olarak şirketimizin herhangi bir hukuki sorumluluğu bulunmamaktadır. Şirketimiz mesajın ve bilgilerinin size değişikliğe uğrayarak veya geç ulaşmasından, bütünlüğünün ve gizliliğinin korunamamasından, virüs içermesinden ve bilgisayar sisteminize verebileceği herhangi bir zarardan sorumlu tutulamaz.
>
> This message and attachments are confidential and intended for the individual(s) stated in this message. If you received this message in error, please immediately notify the sender and delete it from your system. Our company has no legal responsibility for the contents of the message and its attachments. Our company shall have no liability for any changes or late receiving, loss of integrity and confidentiality, viruses and any damages caused in anyway to your computer system.


Thanks for the response!

You suggested that the model should run without any database calls. Do
you mean that they should not exist in a model (ie. a controller should
instantiate /and/ populate a model), or rather that it should be
optional (ie. each method only updates the state of the object, and
load() & save() must be explicitly called to interact with the DB)?

Or am I way off track.

Cheers,
Eric

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On 7/10/2009 7:25 PM, David Otton wrote:
> 2009/10/7 Eric Bauman:
>
>> Any thoughts would be much appreciated!
>
> One observation. "Model" isn't a synonym for "Database Table" - models
> can be anything that encapsulates business logic. Requiring all your
> models to inherit from Model is probably a bad idea.

Thank-you for responding.

Out of curiosity, why is this a bad idea? Is it also bad for views &
controllers?


Cheers,
Eric

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

--0003255574f22a812e0475583176
Content-Type: text/plain; charset=ISO-8859-1

On Wed, Oct 7, 2009 at 8:06 AM, Eric Bauman wrote:

> On 7/10/2009 7:25 PM, David Otton wrote:
>
>> 2009/10/7 Eric Bauman:
>>
>> Any thoughts would be much appreciated!
>>>
>>
>> One observation. "Model" isn't a synonym for "Database Table" - models
>> can be anything that encapsulates business logic. Requiring all your
>> models to inherit from Model is probably a bad idea.
>>
>
> Thank-you for responding.
>
> Out of curiosity, why is this a bad idea? Is it also bad for views &
> controllers?
>
>
> Cheers,
> Eric
>
>
> --
> PHP General Mailing List (http://www.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
It's about responsibilities.
A Model should encapsulate business logic, and business logic and
persistence has nothing in common.
Maybe you can code a loader/persister object and let your models rely on it
to handle the instances?
This way you can decouple you business logic from the data persistence,
allowing you to change one of them without introducing bugs into the other.
Suppose you need to change your Database engine => change the
loader/persister.
You need to add/remove/modify some new rules in your business logic =>
change the models

Why don't you allow the view to use the model?
This way you have...
# the controller is responsible to instantiate the model(s) and view(s)
# the model is responsible for the business logic
# the view is responsible for the presentation




--
Martin Scotta

--0003255574f22a812e0475583176--

RE: Insult my code!

--_3b43937b-9aa6-404f-a7fb-b1c5092cad89_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable


So far I stopped at the first line=2C the constructor=2C where I can spot w=
ith what I can read SQL injections "everywhere"

I hope here is a proper validation there=2C 'cause as is=2C sounds truly da=
ngerous=2C since you are not using bindParams or other PDO related techniqu=
es to avoid input problems.

About the rest I kinda agree with the proper model controller=2C rather tha=
n just a reader.

Regards

> To: php-general@lists.php.net
> Date: Wed=2C 7 Oct 2009 17:34:35 +1100
> From: baumane@livejournal.dk
> Subject: [PHP] Insult my code!
>=20
> Hi there=2C
>=20
> I'm in the process of trying to wrap my head around MVC=2C and as part of=
=20
> that=2C I'm attempting to implement a super-tiny MVC framework.
>=20
> I've created some mockups of how the framework might be used based=20
> around a very simple 'bank'=2C but I'm trying to get some feedback before=
=20
> I go and implement it=2C to make sure I'm actually on the right track.
>=20
> Any thoughts would be much appreciated!
>=20
> Model - http://www.pastebin.cz/23595
> Controller - http://www.pastebin.cz/23597
> View - http://www.pastebin.cz/23598
> Template - http://www.pastebin.cz/23599
>=20
> --=20
> PHP General Mailing List (http://www.php.net/)
> To unsubscribe=2C visit: http://www.php.net/unsub.php
>=20
=0A=
____________________________________________________________ _____=0A=
Windows Live: Friends get your Flickr=2C Yelp=2C and Digg updates when they=
e-mail you.=0A=
http://www.microsoft.com/middleeast/windows/windowslive/see- it-in-action/so=
cial-network-basics.aspx?ocid=3DPID23461::T:WLMTAGL:ON:WL:en -xm:SI_SB_3:092=
010=

--_3b43937b-9aa6-404f-a7fb-b1c5092cad89_--

Re: Insult my code!

On Wed, Oct 07, 2009 at 05:34:35PM +1100, Eric Bauman wrote:

> Hi there,
>
> I'm in the process of trying to wrap my head around MVC, and as part of
> that, I'm attempting to implement a super-tiny MVC framework.
>
> I've created some mockups of how the framework might be used based
> around a very simple 'bank', but I'm trying to get some feedback before
> I go and implement it, to make sure I'm actually on the right track.
>
> Any thoughts would be much appreciated!
>
> Model - http://www.pastebin.cz/23595
> Controller - http://www.pastebin.cz/23597
> View - http://www.pastebin.cz/23598
> Template - http://www.pastebin.cz/23599

Your code (what there is of it) is fine. Beware of people who criticize
your code on purely academic criteria. There are a lot of differing
opinions about MVC, much of it driven by people making academic points
versus people who actually code for a living. Even some people who
actually code for a living fall under the spell of academic rules about
this or that.

The real key is, does it work, and can it be maintained. If so, don't
worry about people who argue esoteric points about what should or
shouldn't be in models and controllers, etc.

Paul

--
Paul M. Foster

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

2009/10/7 Eric Bauman :
>
> On 7/10/2009 7:25 PM, David Otton wrote:
>>
>> 2009/10/7 Eric Bauman:
>>
>>> Any thoughts would be much appreciated!
>>
>> One observation. "Model" isn't a synonym for "Database Table" - models
>> can be anything that encapsulates business logic. Requiring all your
>> models to inherit from Model is probably a bad idea.
>
> Thank-you for responding.
>
> Out of curiosity, why is this a bad idea? Is it also bad for views &
> controllers?

Well, the way MVC has traditionally been approached is that the VC bit
is a thin interface skin, concerned with I/O, while the M is the bulk
of the program - the bit that does the heavy lifting. (You'll often
hear this called "Fat Model, Skinny Controller"). So you could have a
lot of models, that do a lot of disparate stuff - not just database
tables.

To give you an example, imagine an application that texts people when
a new book by their favourite author is coming out.

It's going to have models to represent database tables, the Amazon API
and an SMS API.

If all your controllers assume that all your models inherit from
Model, then your code is monolithic and none of it can be reused.

http://c2.com/cgi/wiki?InheritanceBreaksEncapsulation

Of course, I'm talking about a general-purpose framework here - if all
you're ever going to do is CRUD operations on DB tables, then by all
means make assumptions about your models. You end up with all your
classes tightly coupled, but they're still perfectly fit for purpose.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On Wed, Oct 07, 2009 at 09:09:29PM +0100, David Otton wrote:

> 2009/10/7 Eric Bauman :
> >
> > On 7/10/2009 7:25 PM, David Otton wrote:
> >>
> >> 2009/10/7 Eric Bauman:
> >>
> >>> Any thoughts would be much appreciated!
> >>
> >> One observation. "Model" isn't a synonym for "Database Table" - models
> >> can be anything that encapsulates business logic. Requiring all your
> >> models to inherit from Model is probably a bad idea.
> >
> > Thank-you for responding.
> >
> > Out of curiosity, why is this a bad idea? Is it also bad for views &
> > controllers?
>
> Well, the way MVC has traditionally been approached is that the VC bit
> is a thin interface skin, concerned with I/O, while the M is the bulk
> of the program - the bit that does the heavy lifting. (You'll often
> hear this called "Fat Model, Skinny Controller"). So you could have a
> lot of models, that do a lot of disparate stuff - not just database
> tables.
>
> To give you an example, imagine an application that texts people when
> a new book by their favourite author is coming out.
>
> It's going to have models to represent database tables, the Amazon API
> and an SMS API.
>
> If all your controllers assume that all your models inherit from
> Model, then your code is monolithic and none of it can be reused.
>
> http://c2.com/cgi/wiki?InheritanceBreaksEncapsulation

I think this is a bit extreme. It really depends on what's in your
parent model class. It could be something really simple, but something
you don't want to have to rewrite in every model you code. Thinking that
a model must stand on its own without inheriting from a parent model
class because of some ideal notion of encapsulation is silly.

Actually, in all deference to the eggheads of OOP, I believe there are
really only four reasons to use classes:

1. It keeps me from having to rewrite the same code over and over
(inheritance).

2. It keeps the namespace cleaner, since methods are unknown outside the
context of their containing class.

3. It allows me to change details of function implementation without
breaking things. This is a weak claim, though, because I can do the same
thing with straight functions; just change the guts of the function,
while still providing the same inputs and outputs.

4. It keeps the stack cleaner. I don't have to pass the same crap to
every related function. I can keep the data inside my class and have all
the methods share it.

The notion that OOP was going to save the world billions of hours of
programming time because of re-use is bunk. It hasn't, doesn't and
won't.



Paul

--
Paul M. Foster

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

2009/10/7 Paul M Foster :

> I think this is a bit extreme. It really depends on what's in your
> parent model class. It could be something really simple, but something
> you don't want to have to rewrite in every model you code. Thinking that

Have you got an example of something that is needed by every model
that interacts with a general-purpose framework?

> 1. It keeps me from having to rewrite the same code over and over
> (inheritance).

Inheritance isn't the only mechanism for code-reuse, but it is the
most tightly bound. In some situations it may be the appropriate
solution, but it does force you into a taxonomy straightjacket. You
just need to be aware of that, and pick the appropriate tool for the
job.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On Wed, Oct 07, 2009 at 11:31:58PM +0100, David Otton wrote:

> 2009/10/7 Paul M Foster :
>
> > I think this is a bit extreme. It really depends on what's in your
> > parent model class. It could be something really simple, but something
> > you don't want to have to rewrite in every model you code. Thinking that
>
> Have you got an example of something that is needed by every model
> that interacts with a general-purpose framework?

Probably not a great example, but how about this: *Assuming* that the
models need variables held in the config (which database, if any, for
example), we put code in the constructor of the parent model which picks
up these variables and stores references to them, for use in inherited
models. The config could be a singleton class which holds a single
instance of all the config variables. I *could* include a call like

$this->config =& get_config();

in each model's constructor. Or I could just do it once in the parent
model. Of course, if this is all the parent model provides, then

$this->config =& get_config();

in each model would be roughly equivalent to

parent::Model();

in each model. But if the parent provided more than that, then I would
be writing less code to simply build it into the parent. And of course,
if I ever wanted to *add* more to all the models, then having a parent
model would allow me to do so without having to rejigger the code in
each model.

Paul

--
Paul M. Foster

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

RE: Insult my code!

Hi Paul,

As I agree some of your thoughts, I want to add my opinion also.
Yes the code should work. That is why we earn Money. If it doesnt work, the=
n we are on fire. But things like OOP or MVC weren't invented for a better =
running code. They are invented so the codes will going to be much more cle=
an, readable, reusable, maintainable. "Running codes" is not enough.

Eric asked about how his MVC structure looks. And we are trying to help wha=
t we know about MVC. He didn't asked if the code is fine for running. So gi=
ving an answer "The real key is, does it work, and can it be maintained" is=
not enough and not really helpful to him on MVC concept. If you need just =
a running and maintainable project, you don't need to use MVC (MVC is not a=
ll about that). We are not criticizing his code(the code is really fine(exc=
ept injection problem :-) ) and very readable)


Eric,

As Martin said, All the business logic should be in Model. Controller shoul=
d not tell a model "Save it to this database, select it from this table, us=
e this db_adapter" etc. A controller is like a maestro of the system. It as=
kes the model to play piano loud. But it wont say which key of piano, the m=
odel should touch.

I suggest you to read this online book about Zend Framework and MVC. Its re=
ally really very helpful to understand the concept. Also example codes are =
very clean and good.
http://www.survivethedeepend.com/zendframeworkbook/en/1.0


Take Care,

Mert
(sorry for my english)


-----Original Message-----
From: Paul M Foster [mailto:paulf@quillandmouse.com]
Sent: Wednesday, October 07, 2009 7:54 PM
To: php-general@lists.php.net
Subject: Re: [PHP] Insult my code!

On Wed, Oct 07, 2009 at 05:34:35PM +1100, Eric Bauman wrote:

> Hi there,
>
> I'm in the process of trying to wrap my head around MVC, and as part of
> that, I'm attempting to implement a super-tiny MVC framework.
>
> I've created some mockups of how the framework might be used based
> around a very simple 'bank', but I'm trying to get some feedback before
> I go and implement it, to make sure I'm actually on the right track.
>
> Any thoughts would be much appreciated!
>
> Model - http://www.pastebin.cz/23595
> Controller - http://www.pastebin.cz/23597
> View - http://www.pastebin.cz/23598
> Template - http://www.pastebin.cz/23599

Your code (what there is of it) is fine. Beware of people who criticize
your code on purely academic criteria. There are a lot of differing
opinions about MVC, much of it driven by people making academic points
versus people who actually code for a living. Even some people who
actually code for a living fall under the spell of academic rules about
this or that.

The real key is, does it work, and can it be maintained. If so, don't
worry about people who argue esoteric points about what should or
shouldn't be in models and controllers, etc.

Paul

--
Paul M. Foster

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php


Bu mesaj ve ekleri, mesajda g?nderildi?i belirtilen ki?i/ki?ilere ?zeldir v=
e gizlidir. Size yanl??l?kla ula?m??sa l?tfen g?nderen kisiyi bilgilendirin=
iz ve mesaj? sisteminizden siliniz. Mesaj ve eklerinin i?eri?i ile ilgili o=
larak ?irketimizin herhangi bir hukuki sorumlulu?u bulunmamaktad?r. ?irketi=
miz mesaj?n ve bilgilerinin size de?i?ikli?e u?rayarak veya ge? ula?mas?nda=
n, b?t?nl???n?n ve gizlili?inin korunamamas?ndan, vir?s i?ermesinden ve bil=
gisayar sisteminize verebilece?i herhangi bir zarardan sorumlu tutulamaz.

This message and attachments are confidential and intended for the individu=
al(s) stated in this message. If you received this message in error, please=
immediately notify the sender and delete it from your system. Our company =
has no legal responsibility for the contents of the message and its attachm=
ents. Our company shall have no liability for any changes or late receiving=
, loss of integrity and confidentiality, viruses and any damages caused in =
anyway to your computer system.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On 8/10/2009 1:20 AM, Andrea Giammarchi wrote:
>
> So far I stopped at the first line, the constructor, where I can spot with what I can read SQL injections "everywhere"
>
> I hope here is a proper validation there, 'cause as is, sounds truly dangerous, since you are not using bindParams or other PDO related techniques to avoid input problems.
>
> About the rest I kinda agree with the proper model controller, rather than just a reader.
>
> Regards
>
>> To: php-general@lists.php.net
>> Date: Wed, 7 Oct 2009 17:34:35 +1100
>> From: baumane@livejournal.dk
>> Subject: [PHP] Insult my code!
>>
>> Hi there,
>>
>> I'm in the process of trying to wrap my head around MVC, and as part of
>> that, I'm attempting to implement a super-tiny MVC framework.
>>
>> I've created some mockups of how the framework might be used based
>> around a very simple 'bank', but I'm trying to get some feedback before
>> I go and implement it, to make sure I'm actually on the right track.
>>
>> Any thoughts would be much appreciated!
>>
>> Model - http://www.pastebin.cz/23595
>> Controller - http://www.pastebin.cz/23597
>> View - http://www.pastebin.cz/23598
>> Template - http://www.pastebin.cz/23599
>>
>> --
>> PHP General Mailing List (http://www.php.net/)
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>
> ____________________________________________________________ _____
> Windows Live: Friends get your Flickr, Yelp, and Digg updates when they e-mail you.
> http://www.microsoft.com/middleeast/windows/windowslive/see- it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLM TAGL:ON:WL:en-xm:SI_SB_3:092010

The linked code was supposed to be more of a mockup than anything, with
the functions a bit of filler to try and show what I'm trying to do.

With regard to the SQL injection, I try not to make the problems with my
code quite so blatant. :-)

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On 8/10/2009 5:18 PM, Mert Oztekin wrote:
> Hi Paul,
>
> As I agree some of your thoughts, I want to add my opinion also.
> Yes the code should work. That is why we earn Money. If it doesnt work, then we are on fire. But things like OOP or MVC weren't invented for a better running code. They are invented so the codes will going to be much more clean, readable, reusable, maintainable. "Running codes" is not enough.
>
> Eric asked about how his MVC structure looks. And we are trying to help what we know about MVC. He didn't asked if the code is fine for running. So giving an answer "The real key is, does it work, and can it be maintained" is not enough and not really helpful to him on MVC concept. If you need just a running and maintainable project, you don't need to use MVC (MVC is not all about that). We are not criticizing his code(the code is really fine(except injection problem :-) ) and very readable)
>
>
> Eric,
>
> As Martin said, All the business logic should be in Model. Controller should not tell a model "Save it to this database, select it from this table, use this db_adapter" etc. A controller is like a maestro of the system. It askes the model to play piano loud. But it wont say which key of piano, the model should touch.
>
> I suggest you to read this online book about Zend Framework and MVC. Its really really very helpful to understand the concept. Also example codes are very clean and good.
> http://www.survivethedeepend.com/zendframeworkbook/en/1.0
>
>
> Take Care,
>
> Mert
> (sorry for my english)

Thanks for the link, it looks like an interesting read. Hopefully it
will help me understand MVC better and hence allow me to improve my code
design.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Insult my code!

On 7/10/2009 5:34 PM, Eric Bauman wrote:
> Hi there,
>
> I'm in the process of trying to wrap my head around MVC, and as part of
> that, I'm attempting to implement a super-tiny MVC framework.
>
> I've created some mockups of how the framework might be used based
> around a very simple 'bank', but I'm trying to get some feedback before
> I go and implement it, to make sure I'm actually on the right track.
>
> Any thoughts would be much appreciated!
>
> Model - http://www.pastebin.cz/23595
> Controller - http://www.pastebin.cz/23597
> View - http://www.pastebin.cz/23598
> Template - http://www.pastebin.cz/23599

Hi again,

I've done quite a bit of reading, thinking and coding based on all the
feedback I received from my original post.

I have since come up with this:

Model - http://www.pastebin.cz/23857
Controller - http://www.pastebin.cz/23858
View - http://www.pastebin.cz/23859
Persistence - http://www.pastebin.cz/23860

As before, please feel free to insult my code. ;-) Any and all feedback
is of course most appreciated.

Best regards,
Eric

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Re: Insult my code!

2009/10/11 Eric Bauman :

> As before, please feel free to insult my code. ;-) Any and all feedback is
> of course most appreciated.

I know you're more concerned with structure, but your checkInt()
method is arguably buggy/has an un-noted assumption. It accepts ints
formatted as ints and strings, but not floats:


require_once 'PHPUnit/Framework/TestCase.php';
require_once 'BankModel.php';

class BankModelTest extends PHPUnit_Framework_TestCase
{
function testSetBalanceAcceptsInts()
{
$fixture = new BankModel();
$int = 1351236;
$this->assertNull( $fixture->setBalance($int) );
}

function testSetBalanceAcceptsFloats()
{
$fixture = new BankModel();
$float = (float)1351236;
$this->assertNull( $fixture->setBalance($float) );
}

function testSetBalanceAcceptsStrings()
{
$fixture = new BankModel();
$string = (string)1351236;
$this->assertNull( $fixture->setBalance($string) );
}
}

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Re: Insult my code!

On 12/10/2009 9:21 PM, David Otton wrote:
> 2009/10/11 Eric Bauman:
>
>> As before, please feel free to insult my code. ;-) Any and all feedback is
>> of course most appreciated.
>
> I know you're more concerned with structure, but your checkInt()
> method is arguably buggy/has an un-noted assumption. It accepts ints
> formatted as ints and strings, but not floats:
>

*sigh* sometimes I really wish PHP allowed one to be a bit more
heavy-handed with types (optional real type hinting would be nice).

I guess I only ever worried about string (from DB) and int (internal
call) as in my specific use I would never be passing a float.
You make an excellent point however; I suppose in the interests of
completeness, forward compatibility etc. I should take into account more
possibilities. Perhaps I should just throw an exception in deposit()
etc. if the argument isn't int and worry about converting elsewhere.

Also thanks for the sample TestCase code! I've never really thought
about unit testing in PHP, despite doing so in Java etc. Reading about
PHPUnit brought me on to phpUnderControl - interesting stuff!


Best regards,
Eric

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: Re: Insult my code!

2009/10/13 Eric Bauman :

> *sigh* sometimes I really wish PHP allowed one to be a bit more heavy-handed
> with types (optional real type hinting would be nice).

There's a scalar type-hinting patch floating around somewhere, but
then your code would only work on machines with that patch.

> I guess I only ever worried about string (from DB) and int (internal call)
> as in my specific use I would never be passing a float.
> You make an excellent point however; I suppose in the interests of
> completeness, forward compatibility etc. I should take into account more
> possibilities. Perhaps I should just throw an exception in deposit() etc. if
> the argument isn't int and worry about converting elsewhere.

I can see two choices - either throw an exception in checkInt() if
it's passed anything except an int or a string (which is fine -
because you've made the assumption explicit) or change the checkInt()
so it deals with ints-masquerading-as-floats. Try:

function checkInt($x) {
return((string)((int)$x) == $x);
}

> Also thanks for the sample TestCase code! I've never really thought about
> unit testing in PHP, despite doing so in Java etc. Reading about PHPUnit
> brought me on to phpUnderControl - interesting stuff!

Yup. Pretty graphs. That's not the best unit test example to work
from, BTW - the manual will give you better advice than I.

--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php