Dependency Injection Breaks Encapsulation

[cpradio]

To prevent this from jumping down the rabbit hole again, let’s not focus on trying to teach Tony (or whomever), or focus on their their actual implementation versus what was read out of an article (as let’s face it, an article can be considered theory, the code starts to make it borderline personal/factual).

Let’s keep the topic on a more theoretical level. I know we’ve had a bit of fun poking at the implementation of Radicore, but I think we can all agree that our time would be better spent talking about the theoretical shortcomings versus making Tony feel defensive as we point to his code (and we all have already been there once or twice).

So to jump back to the theoretical dimension, let’s look at the following statements that were made recently and work on backing them up (without involving another person’s codebase – unless you have their permission, or feel free to share your own).

@s_molinari

a class with a lot of if/ elseif/if /elseif statements is another good sign of SRP not being followed

So I agree with this but I’m curious as to what techniques you’ve used to abide by that statement. How did you limit your if/else statements? (and no, I haven’t watched the video, so if you used the techniques expressed in it, I’ll get to watching it shortly )

Let’s also keep discussing

@s_molinari

Which belong in your MySQL and Oracle abstractions and not in your default table class.

and

@s_molinari

Which, from a database abstraction logic standpoint, also has absolutely nothing to do with a default table class.

These are two points I feel are true on a theoretical level too. Given a set of databases you must support, how would you accomplish it? Assuming you have to abide by the following:

• Must be able to connect to multiple databases within the same page load
• Must be able to connect to multiple databases across transactions (along with knowing when to commit/rollback)
• Must be able to combine the data from multiple databases into a view/model

[s_molinari]

@cpradio

How did you limit your if/else statements? (and no, I haven’t watched the video, so if you used the techniques expressed in it, I’ll get to watching it shortly )

Oh, I also have a hard time with reducing if/ else statements too. They are just so easy to write and basically follow KISS really. However, please do watch the video, as the points Raphael make are quite interesting and he demonstrates them in practical scenarios. I wouldn’t have mentioned the video, if he hadn’t.

Edit: I’d like to go into refactoring Tony’s work more in depth, but I am going to use the excuse that I have to register on his site to download his framework and I don’t really want to do that.

Scott

[cpradio]

So I finally got a chance to watch the youtube video and not too surprising, we actually follow Rules 1 and 2 frequently in our enterprise application.

We don’t have massive if/else blocks, or loops, embeded in a single method. It makes it way to hard to write a test for and typically we write our tests first (as best as we can).

Rule 3 is interesting. Really not useful to our environment though, as we’re a .NET shop, so we all use Visual Studio. To wrap a true/false indicator is a major waste as we have VisualStudio and ReSharper already providing hints as to what that method is doing. We don’t have to go looking for it.

However, for other scenarios, I do agree with it and we have done it. It pretty well falls down to, can what is being passed in be utilized in various ways, if so then a DI approach makes more sense and you should pass in the object to be utilized versus a list of parameters.

Rule #4, we sort of utilize, but with .NET it sort of puts you in a process of wanting to use multiple dots. Example: myObject.ToList().OrderBy(a => a.Name) Granted, that isn’t the best example, as it is chaining properties, and they are methods, so maybe the rule taking in mind of properties only, does make sense.

I can’t tell you how many times I’ve fixed code that was chaining methods only to have a piece in the middle return NULL and blow up the entire thing. So I definitely get the idea behind the rule, but it is one of those “where it makes sense” and to some extent, you have to know what can be returned to make that determination.

Rule #5, I tell this to everyone ALL of the freakin’ time! Especially in .NET it is pointless, especially in .NET where everything is compiled to MSIL. The compiler will optimize the variable names for you, you don’t have to worry about it. I do like the thought of, if your method is very long, chances are you are doing too much in it!

Rule #6, We do this. We rarely have a class that becomes very large and when it does, we look at it to see why? Many times it is because it is doing more than it should and we split it apart appropriately.

Rule #7, There are a lot of good points in this one.

Rule #8, We do this. Our collections are purely collections with only operations dealing with the collection. The collection eventually gets passed back to its service to process update, inserts, deletes, etc.

Rule #9, Interesting. I’ll have to research what C# has to replace accessors with. For the most part our properties do not do more than just return the data, so we really don’t need accessors, we simply use protected string name;, I can only think of a few rare cases where an accessor was necessary.

Rule #10, Couldn’t agree more. I’d say 90% of our code base right now is self documenting, but the parts that were hard to describe the intended action, or have had a lot of bugs related to it, have comments so the next person understands what it should be doing (as too many people have come in after the fact, changed it to what they thought it should do, and broke it).

[TomB]

Jeff posted this in the other thread and it’s totally relevant. Tony keeps discussing things in terms of how it affects his existing code instead of in the terms of a choice someone developing a new piece of software would make.

@Jeff_Mott

So, for the purpose of comparing implementation choices, I suggest to everyone in this topic that we do so in the context of a brand new application. That is, if we were to sit down and write a brand new application, would we start off using require statements or an autoloader? Why or why not?

So tony, on those terms, if someone is making a brand new application, why should they choose a singleton over DI? My guess is this is a question you cannot answer without falling back into labelling DI “inappropriate”.

Please answer it in the manner I described here:

Dependency Injection Breaks Encapsulation

You know what tony, I’ll help you out in order to try to progress this discussion in a meaningful way. As you’re finding it so incredibly difficult to make your own point, I’ll try to steer you towards something that isn’t meangingless by giving you a 3 step template to follow. I’ll restate my argument in the manner I’m asking you to, I think that’s only fair. (I have done this several times already but hopefully another demonstration will help). Note that all code examples are complete, can be…

[cpradio]

@TomB

why should they choose a singleton over DI?

I’ve only had one time where I chose a singleton over DI. I needed to wrap a third party utility and that utility was insistent that it only have one instance running at a time (or bad things would happen). Could I have used a DI approach combined with another to ensure a single instance was always provided? Yep.

I am curious how others might solve that need with what exists today. As I had that need nearly 7 years ago and haven’t had it since.

[tony_marston404]

@s_molinari

You are an expert of your own framework, which I gather very few people have the want to master. The reason nobody wants to master it or use it is because you are NOT an expert at OOP.

My framework is only used by people who want to write enterprise applications as used by big corporations, not public-facing web sites.

Nobody can class themselves as an expert at OOP simply because the programming community cannot even agree on what OO is, let alone the best way to do it.

For the record, I have never claimed to be an expert at OOP, merely less incompetent than most.

@s_molinari

I have a beef that you come to forums like this one or LinkedIn, where I got to know you first, and you make arguments nobody really wants to hear and you make arguments, which might lead the unwitting to think wrongly about OOP methodologies.

The fact that you don’t agree with the arguments that I make does not mean that those arguments are automatically invalid. For every person who expresses an opinion there will always be hundreds of others who hold a different opinion. That is the nature of a free society, so learn to live with it.

As for making people “think wrongly about OOP methodologies” I prefer to say that I am encouraging them to think differently. I am encouraging people to think about what they are doing and why instead of blindly following a practice simply because someone tells them to.

@s_molinari

In some ways, I get the feeling you actually know you are wrong in your actions and thinking, deep down inside, but the big hairy monster called Radicore Framework causes you to be like you are.

I know no such thing.

@s_molinari

you’d have to do your OOP differently…do OOP properly.

Define “properly”. As there are many different opinions as to what OO actually is, there are as many opinions as to how to implement it. If there are multiple ways in which a concept can be implemented, who is to say which is “proper” and which is not?

@s_molinari

It will be a way to learn and constantly expand your knowledge, which is what a community is for.

You mean a community in which only the established views are allowed to be voiced, and all dissension is quashed?
.
Post edited by cpradio to remove the personal quarrel

[tony_marston404]

@TomB

This is a very good point. Tony, this is a serious question: If you were starting a new project that wasn’t based on RADICORE, maybe a e-commerce website or something else that’s not suited to it, would you use things like autoloaders, visibility modifiers, unit testing, php5 constructors, namespaces, DI and all that other stuff you call “optional”?

No. Simply because I do not work on public-facing websites, I deal with enterprise applications as used by large corporations. Such applications grow and mature over a long period of time and are not simple little things that you can throw together in a matter of weeks or months.

@TomB

I’ve always come to the conclusion that the reason tony is so argumentative is because he thinks if he can justify his bad practices to us (or others) and people agree with him, he doesn’t have to admit his code is awful by any standard.

Just because you choose not to write code in the same style as me does not automatically make my style bad and your style good. There is no single style which is universally accepted as the only style which people are allowed to follow, so I will follow whichever style allows me to develop software which my corporate customers are willing to buy.

@TomB

Some of the code I wrote 10 years ago is as bad as Tony’s (No 9000 line classes, mind, but all the other stuff with mixed responsibilities, procedural code shoe-horned in to classes, global state, etc… however, I was 18 with no real formal training or experience!), the difference is I am happy to admit it was bad and can highlight how I’ve improved as a developer and you know what, if I hadn’t written it, I wouldn’t have learnt from it. Tony, on the other hand, once he’s written something it seems the ideas within it are then set it stone and can never be changed or even questioned.

You fail to realise one significant point - when I switched to PHP as my language of choice in 2002 I had already had over 20 years’ experience with other languages. During that time I learned the following:

• that modular programs are better than monolithic programs.
• that well structured programs are better than badly structured programs.
• that with database applications the most important part is the database design.
• that programs should be structured around the database design and not the other way around.
• that program code needs to be readable and understandable by humans more than they need to be executed by a computer.
• that readable code requires the use of meaningful variable and function names.
• that simple code is preferable to complex code (this is the essence of the KISS principle).

I have come across many different ideas in my long career, but where an idea has failed I do not waste my time in trying to repeat it in the hope that this time it will be better. Where an idea has worked for me in the past I will continue to use that idea, but always with an eye out to find improvements if possible.

One thing I will not do is bow to peer pressure. If I think something is wrong then I will have nothing to do with it. If I have a method which has worked successfully for many years then I will stick with that method and not waste my time in ditching it in favour of some wild idea that has been dreamt up by some other programmer.

Post edited by cpradio to remove negative connotation of others involved in this topic

[TomB]

@tony_marston404

No. Simply because I do not work on public-facing websites, I deal with enterprise applications as used by large corporations. Such applications grow and mature over a long period of time and are not simple little things that you can throw together in a matter of weeks or months.

Why wouldn’t you use DI? Let’s ignore the other stuff for now and stay on topic: Why wouldn’t you use DI on an new project, what would you use in its place and why?

@tony_marston404

You fail to realise one significant point - when I switched to PHP as my language of choice in 2002 I had already had over 20 years’ experience with other languages.

Lets get away from your code in particular and focus on the underlying issues, please just answer the above question:

Why wouldn’t you use DI? Let’s ignore the other stuff for now and stay on topic: Why wouldn’t you use DI on an new project, what would you use in its place and why?

Please do this with code examples and a bullet pointed list of reasons for your choice. Words like “Not appropriate”, “not designed for” and others are not a suitable reason, you must demonstrate this with something quantifiable.

Post edited by cpradio

[s_molinari]

@tony_marston404

Define “properly”.

We defined proper OOP earlier. Modular, reusable, understandable and testable code.

Scott

Post edited by cpradio

[tony_marston404]

@saamorim

A few examples of the current problems, in my vision of a framework:

• commit, rollback - What if I want to commit/rollback a transaction involving more that one entity ? In which entity should I use this? A random one, all? And if all, what if one fails?
• post_fileUpload - Why should I care of this in the “database” layer? And if I don’t support file upload on a “Car” entity?
• getLanguageArray - I don’t care about multilanguage, and isn’t this a front-end responsibility?
• customButton - Again isn’t this front-end responsibility? Does it make sense use it in batch program? If not, what it is doing here?

You obviously do not understand the relationship between the Model, the View and the Controller. I have 40 page Controllers which are used to provide over 2,000 user transactions, with a separate Controller for each Transaction Pattern. Each of these Controllers calls a different combination of methods on its current Model, and each Model class inherits a great deal of reusable code from my “monster” abstract class. Every method used in a Controller is defined within my abstract class so that it never has to be defined manually in any concrete Model class.

So, if my 40+ Controllers use a total of 121 different methods when communicating with a Model, it should be obvious that my abstract class is required to contain those 121 methods. Each of these public methods usually involves executing a series of internal methods (as shown in the UML diagram which you will find on my website) some of which have a “_cm” prefix which means that they exist in the abstract class as empty methods, but which can be copied to a concrete class and filled with code that will be executed at runtime.

While it is true that my 40+ Controllers use a total of 121 different methods, it should also be obvious that no single Controller uses anywhere near that number of methods. There is no rule that says I should create a separate Model class for each Controller that only contains the methods used by that Controller, so it means that each Model class may contain methods which are not used by several Controllers, but which are used in at least one.

As for your list of questions, I shall answer them individually.

@saamorim

commit, rollback - What if I want to commit/rollback a transaction involving more that one entity ? In which entity should I use this? A random one, all? And if all, what if one fails?

Your knowledge of SQL is obviously not up to scratch. It does not matter how many different tables I access within a single transaction as they all go through the same database connection. While a COMMIT or ROLLBACK needs to be issued by the Controller through a Model it does not matter which Model as the database transaction covers any number of tables, not just one. When I issue a COMMIT it will commit all outstanding updates to all tables. If there is any sort of failure then a single ROLLBACK will undo all of those changes.

@saamorim

post_fileUpload - Why should I care of this in the “database” layer? And if I don’t support file upload on a “Car” entity?

None of my Model classes exist in the “database” layer, they only exist in the “business” layer. The “database” layer is where my DAO exists.

If you don’t support file upload in the “Car” entity then don’t create a user transaction which attempts a file upload into the “Car” entity.

@saamorim

getLanguageArray - I don’t care about multilanguage, and isn’t this a front-end responsibility?

I DO care about multi-language, and no, this is not a front-end responsibility. Text in the user’s current language is loaded into the Model class, then given to the View for formatting into HTML, PDF or CSV. It is not the View’s job to obtain or translate any text.

@saamorim

customButton - Again isn’t this front-end responsibility? Does it make sense use it in batch program? If not, what it is doing here?

While a front-end screen may contain a custom button, the Model needs to have code which does something should a custom button be pressed. The Controller for a batch program will never call the “customButton” method as a batch program does not have a screen.

@saamorim

This is all plain wrong, any system/component can abstract another. So I can have an entity on my code (Person) that abstracts more that one table in the ER (Login + PersonDetails + AddressInfo + what ever I want) for many reasons: performance, obscure complexity, … I can have a many-to-many-to-many-to-many relationships.

I disagree completely. For your information I have implemented the PARTY database as described in Len Silverston’s Data Model Resource Book which contains (among others) a PERSON entity. This database contains 56 tables and 135 relationships, and each table is represented by a different Model class. Each user transaction uses a particular Controller to talk to a particular Model (or Models) in a particular way, My PARTY subsystem now has over 400 user transactions, and these manage to do everything that my customers require.

The idea of having a single PERSON class to cover all the tables that may be used to hold person data is not something which I am willing to contemplate.

[TomB]

And lets not pretend that the statement tony made about “cannot even agree the best way to do it” is at all accurate, it’s disingenuous at best. There is agreement on the fundamentals: Favour Composition over Inheritance, smaller classes with fewer responsibilities are better than large god classes, good code is testable, portable, reusable, easy to understand. Bad code is hard to test, difficult to reuse without changing, inflexible and hard to understand.

While it’s true that there is some discussion about how best to achieve the above, tony’s stance that “Nobody agrees on anything” is total drivel and doesn’t go anywhere to help him claim that a 9000 line class is a good idea.

Post edited by cpradio

[s_molinari]

@tony_marston404

If I think something is wrong then I will have nothing to do with it.

This is fine Tony. And, you really should also stay quiet with those views too or at least voice your opinion in a way that leaves the door open to further reasoning. You literally mouth off about DI being “evil” and constantly point everyone to your framework to show why. You don’t leave the door open to any discussion about the merits of using DI. Let’s start talking shop without mentioning your framework. Put yourself in the other dev’s shoes for a moment and try and answer Tom’s questions.

Scott

[s_molinari]

@tony_marston404

This database contains 56 tables and 135 relationships, and each table is represented by a different Model class.

@tony_marston404

None of my Model classes exist in the “database” layer,

No model classes exist in the database layer, yet each of the 56 database tables are represented by a Model class?

Like I said, we need to get away from your framework and talk shop without it.

Scott

[tony_marston404]

@felgall

To be able to meaningfully discuss anything that depends on OOP requires that each of the people in the discussion actually understand the concept and purpose of OOP.

This is what I wrote back in 2006:

[quote]
Object Oriented Programming is programming which is oriented around objects, thus taking advantage of Encapsulation, Polymorphism, and Inheritance to increase code reuse and decrease code maintenance. [/quote]
My code is oriented around objects, and it uses Encapsulation, Polymorphism, and Inheritance to increase code reuse and decrease code maintenance, therefore it most definitely is OO. It it does not fit your definition, then have you considered the prospect that your definition is at fault?

Post edited by cpradio

[TomB]

@tony_marston404

My code is oriented around objects, and it uses Encapsulation, Polymorphism, and Inheritance to increase code reuse and decrease code maintenance, therefore it most definitely is OO. It it does not fit your definition, then have you considered the prospect that your definition is at fault?

Can we please move past “My code”, “My framework”. Nobody cares about your code. If you want to make a point, do it without citing specific examples and discuss it conceptually.

Any post mentioning specific implementations rather than the concepts being discussed is not helpful for this discussion.

@tony_marston404

And you do? Who is to say that your personal definition of what OO is or is not agrees with everyone else’s definition? As far as I am concerned far too many of today’s programmers are trying to make OOP more complicated than it need be for no other reason than to hide their own ignorance on what is supposed to be a simple concepts. This is what I wrote back in 2006:

This is such drivel. “What’s to say that your personal definition of what the + operator is not agrees with everyone elses definition?” From this viewpoint you can argue 2+2=5 because your interpretation of the + operator differs.

[tony_marston404]

@s_molinari

I have to ask the question that was asked earlier again, what does the default table class represent? What single behavior does it do well? Please state that in one simple sentence.

The default table class is an abstract class which is inherited by every Model class, and which contains methods which may be used by any of the Controllers in the execution of a user transaction.

Is that sentence simple enough for you?

[TomB]

@tony_marston404

The default table class is an abstract class which is inherited by every Model class, and which contains methods which may be used by any of the Controllers in the execution of a user transaction.

The universe is an abstract set of laws which is inherited by all galaxies, which contains stars which may be used to light planets in the execution of the big bang.

You can use this kind of ad-hoc reasoning to explain anything, problem is there’s no way to say the universe is a single responsibility. What you didn’t do at all is explain what the class actually does.

[tony_marston404]

@s_molinari

tony_marston404:
Incorrect. My framework also deals with PostgreSQL and SQL Server, but do you see any code there which is specific to those two? No, you do not. The only reason that the above code is there is because there are different class files available depending on the version of PHP which is running.

Which belong in your MySQL and Oracle abstractions and not in your default table class.

I disagree. There are two possible classes when dealing with MySQL and Oracle databases due to each of them having two different extensions. I therefore have to decide which extension is relevant BEFORE I load the class which deals with that extension.

[s_molinari]

@tony_marston404

The default table class is an abstract class which is inherited by every Model class, and which contains methods which may be used by any of the Controllers in the execution of a user transaction.

Is that sentence simple enough for you?

That doesn’t tell me the responsibility it fulfills in terms of logic or data within your application. All your answer says it that the default table class is a container for methods used by other parts of your framework. In other words, I can’t decipher for what single reason I would want to change that class. Thus, it isn’t following SRP.

Try again!

Scott

[tony_marston404]

@s_molinari

tony_marston404:
With this technique I can access different databases on different servers within the same user transaction

Which, from a database abstraction logic standpoint, also has absolutely nothing to do with a default table class.

You are missing the point. My abstract table class exists in the business layer and does not communicate with any particular database. This makes it database-agnostic as I can switch from using one DBMS to another having to change a single line of code in any component in my business layer.