pradosoft / prado

Prado - Component Framework for PHP
Other
186 stars 71 forks source link

THtmlElement cannot have subclasses define the default #256

Closed ctrlaltca closed 11 years ago

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 01:25:51

Any subclass of THtmlElement will have the default tagname of 'span'

Subclasses should be able to change the default tagname to their own specification r2807

Original issue: http://code.google.com/p/prado3/issues/detail?id=255

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 16, 2010 17:18:34

Honestly, that modification is completely pointless in this form, as when you're actually subclassing THtmlElement, then you can override the getTagName() class and change it to whatever you want. So subclasses already had the "ability to change the 'default' tagname to their own specification". Not that it would make any sense, as if you're creating new controls you're supposed to derive from TControl or TWebControl, not from THtmlElement. THtmlElement is a control created for the sole purpose of enabling inclusion any html element as a control (isntead of as a literal) in the templates. Or if you understand it this way better: use of THtmlElement is only warranted and only makes sense if you're actually setting its TagName property (in which case having a "default" tagname is pointless and superflous).

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 16, 2010 21:24:52

This is extremely useful as it defines the functionality of universally being able to switched the tagname. You don't need to re-implement each setTagName if you are providing your subclassed getTagName. If you want to give a control a specific tag then you can implement TWebControl and thus no need for THtmlElement. If you want to be able to mutate the tag (eg. via skin), you need this to be a universal html mutator and within your specific subclass->getDefaultTag can thus give the default.

This change allows the THtmlElement to provide get/setTagName services which you, as a subclass, don't need to override.

The use of THtmlElement makes no sense if you are always setting the TagName because you may want defaults.

I'll check in some code I made a few months ago that makes use of this. The best example i have is a THeader1.
It extends this new THtmlElement and has the getDefaultTagName() returning 'h1'. In a skin you port from wordpress the h1 may be totally fugly so you want to use the h2 instead. So you set '<com:THeader1 TagName="h2" />' in your theme skin. This will mutate all your coded THeader1s into h2 and thus without any effort make your website beautiful.

I was able to port 2 of the top ten Wordpress themes into my website (both perfectly, i might add) in under 2 hours with this. My templates and pages were all using THeader4s and one theme needed that "blog header" to be h2 to make the headers look good and the other was h3. So I mutated my controls derived from the THtmlElement within the skin. I didn't have to change code on a page, a template control, or my database template control. All THeader4s became what they needed to be to make it look...... awesome.
abilityspaces.org for a working example of one of the themes. I changed the theme to fit our needs so it's not exact.

Lastly, It's backward compatible.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 03:21:11

"If you want to be able to mutate the tag (eg. via skin), you need this to be a universal html mutator and within your specific subclass->getDefaultTag can thus give the default." I don't really get why you would need a mutable tag for skinning, as using CSS actually any tag can be transformed into a different one. You can make a span from a div and vice versa, you have a div display an image like a div, you can even make spans behave like cells of a table. No need to change the tagname for that purpose.

Also, what you're talking about is a functionality specific for a CMS something or something. It's completely fine with me for you to create a specific subclass with that functionality for that purpose (especially at the application-level), but I see no point in altering a base subclass (like THtmlElement which was created specifically to be a general wrapper for a template-specified HTML element) to be altered and extended for that purpose.

If you do that you could just as good make the tagnames of all base-controls mutable (which is complete non-sense, of coure, just like doing that to the base THtmlElement), or add DefaultCssClass, DefaultVisible and default for all the other standard proprerties as well to THtmlElement.

Prado framework base classes shouldn't be abused and extended for application- specific purposes, that make no sense for all (other kind of) applications, as that will only bloat the framework itself, with little gain to most of its users. If you need some added functionality you're always free to create your own subclasses - derived from the base classes - and use those in your application, or use that as a base class for more specific control classes in your application.

"I was able to port 2 of the top ten Wordpress themes into my website (both perfectly, i might add) in under 2 hours with this. " You'd have been able to do the same thing also if you'd not have touched the base THtmlElement but would have simply created your TMutableHtmlElement class (preferably in your application repository, as I think this is very application- specific, or if you can't be talked out of wanting to commit it to the Prado base, then in the Prado repository, but as a separate file/control) and use that or classes derived from that in your app.

Hope you get my point. I'm not against the technique or approach you've invented here - I'm just against bloating the base Prado framework classes with functionality that only or mostly only makes sense in the context of your CMS - or for that purpose in any other specific application - but has little or no gain for any other kind of website, and is also perfectly well implemented in application-specific classes.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 07:43:45

One reason why you'd want to mutate a tag for skinning is if you are importing a theme from another system. If the theme you are importing uses h2 instead of your coded h3. I believe it is insanely useful to able to mutate that tag used throughout the system without touching the code. You could muck with the theme CSS your porting over to match your your system. This could take a lot of work, searching, editing, and ensuring the CSS is properly converted from tag to tag. Or you could just tell all your h4s to be h2s in a skin and the problem is solved.

Just because a class was specifically created for one purpose doesn't mean that its use can't be expanded. Call me crazy, I don't think every child of THtmlElement should reimplement both getTagName and setTagName to get a changable html element with it's own default. That methodology seems redundant to me and adds unwanted and unneeded bloat to every single useful child derived from THtmlElement.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 08:28:06

"One reason why you'd want to mutate a tag for skinning is if you are importing a theme from another system. " The problem is, that Prado is not providing any kind of support for importing a kind of theme you're talking about, nor does it support skinning of this kind. Therefore unless such a system is implemented your extension serves no purpose other than specific support of your CMS, in which case it simply doesn't belong in the Prado core.

And even if you were to implement such a skinning system, then this behaviour should be implemented in the most rudimentary web control class (not in THtmlElement), so it will be available and applicable to all controls.

"If the theme you are importing uses h2 instead of your coded h3. I believe it is insanely useful to able to mutate that tag used throughout the system without touching the code." That's only true if your pages are composed of THtmlElement elements only - which is obviously the case with your CMS, but isn't ever true for any other kind of real Prado application (because those are using lot of other, non-THtmlElement-derived controls too). Extending THtmlElement the way you did will still leave all other standard Prado controls - and their descendants - unskinnable (for ex. you won't be able to mutate TList, TDataGrid, etc. elements, as they're not derived from THtmlElement), therefore usefullness of your implementation is seriously doubtful and limited for the framework as a whole, even if I'd agree with you about the the need for such a system in the framework core.

"Just because a class was specifically created for one purpose doesn't mean that its use can't be expanded. " Indeed, but application-specific extensions have no place in the Prado core base classes. This is a feature that's easily implemented in a derived class, without having to rewrite large chunks of inherited code. Therefore it should be implemented in a separate, derived class.

"Call me crazy, I don't think every child of THtmlElement should reimplement both getTagName and setTagName to get a changable html element with it's own default." No. As I proposed, you should simply create a TMutableHtmlElement intermediary class which you can derive all your other controls from in your CMS. But shouldn't modify THtmlElement itself as outside of your CMS the code added by you serves no purpose, because as I already told you if someone is using THtmlElement in their applications (other than the purpose of a general placeholder in a CMS-like system) then they will fill in its TagName property too, which renders having or being able to set a default tag name useless.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 09:25:20

You are correct Prado does not provide this functionality yet. Yet being the operative word.

I don't think you read my comment thoroughly. I wrote

I believe it is insanely useful to able to mutate that tag used throughout the system without touching the code. << I am referring to an h3 when I say "that tag". Not all tags. You're right. Mutating all tags would be stupid and that's why I didn't do it that way.

While I'd like to describe my grand plan specifically to you, that would take too long. I believe you will start to see the usefulness of this tag and the slight tweaks I've made (while preserving backward compatibility) as I make future checkins down the line that depend upon this. Some of my changes, at least up front here, may not be immediately obviously useful. You'll see these things clarified with future checkins.

You should speak up as you have. It is very useful to have community members participate as you do! (iow, thank you!)

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 17:12:36

"I believe you will start to see the usefulness of this tag and the slight tweaks I've made" I already see it. My problem is that to me it seems it's usefulness is limited to CMS-like applications, in which case they shouldn't be part of the Prado core as they will only result in bloat and longer run-times for any other application. Which is a real problem with Prado anyway. I submitted a lot of improvements to Prado in order to reduce page generation times and thus make it more suitable for high- traffic sites, and I have a real problem with somebody doing quite the opposite, just to satisfy own specific needs with the framework, especially if that could be done just as well without sabotaging common practices and other framework priorities (like reducing bloat and run-time).

Tell me why on earth can't you just create a derived TMutableHtmlElement class and use that instead of THtmlElement whenever needed in your CMS?

Of course I'm not worried about the 10 lines of extra code in THtmlElement per se, but about the basic concept of introducing application-specific code to the framework core, which 1. most likely nobody else will and can use ever (unless they happen to write a CMS and adopt your skinning approach), 2. results in unneccessary bloat and dead code for most uses (as will be the case in any project using THtmlElement right now), 3. goes against common OOP practices like encapsulation and modularization.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 18:17:36

Tell me why on earth can't you just create a derived TMutableHtmlElement class and use that instead of THtmlElement whenever needed in your CMS?<<

If you can tell me how TMutableHtmlElement and THtmlElement are functionally any different then yes. I will change this back and add your proposed tag.

You're telling me that if you wanted to reskin your application without recoding it that this is not useful? This skinning is useful outside the context of a CMS. I've had to reskin 3 PRADO projects. It sucked so hard. This would have been SO, SO handy to not have to retool all my templates. Sometimes I think it would have been faster to write this code and do the reskinning that way rather than retool all my templates!

  1. regarding THtmlElement: PRADO will use this code in the future. Thus it's not application specific. Much of what I'm adding, I'm hoping, you'll see their usefulness in due time with further checkins. 2. javalizards shoot dead-code dead. You are right. All current THtmlElement sub classes have now suddenly become bloated with this code change.
    The prior implementation forced unnecessary re-implementation (aka. bloat) for a different default tag. This pattern is used in PRADO. TControl-Children::getClientClassName is a good example. I thought you were against bloat? 3. I believe my 15 years of OOP are sufficient to say that I'm doing ok.

To reiterate, THtmlElement reduces bloat for any classes that extends it. If you can't see that, I don't know what else to say.

I've gone over a number of your tickets: awesome finds. Thank you for your efforts.

Lastly, just describe how TMutableHtmlElement and THtmlElement are functionally different and you got it. Pretty simple, eh?

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 19:06:34

"If you can tell me how TMutableHtmlElement and THtmlElement are functionally any different then yes. " I already told you that what you did with this was adding dead code for everybody else, except for your own CMS. Because whoever used THtmlElement in their code they specified a TagName property for it - which renders having a DefaultTagName attribute superflous, as the latter's value won't be ever considered, because of having an explicit TagName set.

I also already explained to you earlier why adding a theming system which would only applicable for THtmlElement derivants wouldn't make much sense for Prado as a whole, either.

I understand (and always did, from the beginning on) your need for a generic mutable html wrapper for content elements in your CMS. It's a great idea for make it even more flexible. The problem is not with that. The problem is, that what you've added to THtmlElement makes very much no sense outside of the content area of a CMS system, and is completely usesless without having a theming system in-place which supports it. That's why it doesn't belong into Prado core.

"The prior implementation forced unnecessary re-implementation (aka. bloat) for a different default tag. " Wrong. You only have to create an intermediary TMutableHtmlElement to derive all your specific classes from (like THeaderHtmlElement, TParagraphElement or whatever I guess you already created for your CMS). That's exactly what I'm proposing to you, and explaining now for the 10th time in a row. If you'd just do that you wouldn't need to reimplement mutation support in each of your subclasses and still wouldn't unneccessarily bloat the Prado core for everybody else. Actually, the only thing you'd have to do is to change them to inherit from TMutableHtmlElement instead of THtmlElement itself - and you'd not have to change anything else in them.

"You're telling me that if you wanted to reskin your application without recoding it that this is not useful? " Yes. Without having a theme manager doing the reskinning for you it's useless. Also, if you didn't or don't build your pages with THtmlElements or derivants all over them, it's useless (because your modification obviously doesn't enable me to skin any non-THtmlElement-derived controls directly).

"To reiterate, THtmlElement reduces bloat for any classes that extends it." Nope. That would only hold true for classes and applications that are derived from it AND want/use mutable element support. Right now that's only your CMS and without a proper theme manager added to Prado will always stay only your CMS. (But as already explained, if Prado was ever to receive a proper theme manager, theme support should be implemented in the most rudimentary control class, not in THtmlElement). That's why it's dead code for everybody else.

"3. I believe my 15 years of OOP are sufficient to say that I'm doing ok." Sorry, but that's not an acceptable argument at all. Someone being older, bigger, etc. doesn't neccessarily make their actions or creations techically superior by any means. Just like your 15 years of OOP experience doesn't make your inefficient and unclean patches efficient and clean.

Actually, whenever someone starts to brag with such things then you can know that they ran out of real arguments to bolster their opinions and validate their choices. I actually have +5 year on you in OOP, still I'm not using that as an argument, as you could see, but was explaining why you did things wrong. That's what I'm expecting from you too if you want to "defend" your opinion, instead of telling me how old, tall you are, or how long you've been doing this or that.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 19:46:14

That would only hold true for classes and applications that are derived from it AND want/use mutable element support<<

Then why even have setTagName? If you don't want it to mutate use TWebControl. It sounds like you've been using THtmlElement when you should be using TWebControl.

Due to the fact that you haven't listened to me, and your perpetual claims that this is not being used in PRADO in any way: r2813

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 19:58:00

I already told you<< There are a LOT of things you have alluded to but not really explained or explained very well.

Now it is being used by PRADO, for the added classes it's not bloat, and It's backward compatible. problem solved.

Also, Have you seen the TThemeManager and how it skins? If you don't like how it works, you should open a new issue a describe how it should work. I'd love to hear your ideas. I have some ideas on sub-theming and allowing modules to have their own options but I haven't coded anything yet.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 21:30:35

"Then why even have setTagName? If you don't want it to mutate use TWebControl. " THtmlElement is there so you can represent any html element as a control in a template. That's why it has TagName property so you can specify what HTML element you would like it to represent / render as. TWebControl on the other side doesn't provide this functionality - you'd need to subclass it in order to represent any other element than a 'span' with in. The difference between THtmlElement and a TWebControl-derivant with an overriden getTagName() method is that for the latter you'd need to define a new class, whereas with the former you don't need to write any code, just place it in the template. That's the sole reason why THtmlElement is there - to save the need to define and write a subclass.

Sorry, but with every new post you just make it more clearer that you obviously don't really know Prado and don't get a grasp on most of the concepts employed in it, or why things are done the way they are. Under these circumstances I have to press even harder for you not committing any serious changes to the repository without asking for comments first.

"Now it is being used by PRADO, for the added classes it's not bloat, and It's backward compatible. problem solved." No, you didn't solved any of the problems I pointed out, you just fled into total ignorance. You extension is still a bloat and completely pointless for the base THtmlElement class.

Also sorry but your new controls make no sense at all. You added no functionality in those controls, rendering them completely pointless to have them as separate control classes. For ex. is completely pointless because you could achieve the same result with (which would be just as suitable for skinning too). If you add THeaderX you could just as good add TRedDiv, TBlueDiv which have the Style attribute set to "color: red" and "color: blue" by default. And don't even stop there - add TRedDivWithFontSizeOf12AndAlignedRight too, because someone might need a div with a font-size of 12 and aligned right somewhere in their templates, in which case your new supercontrol could come just handy for them. Right?

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 17, 2010 23:14:43

I'm going to try a different approach: I'm going to copy your lines, then my reply.

"Then why even have setTagName? If you don't want it to mutate use TWebControl. " THtmlElement is there so you can represent any html element as a control in a template<< ok good. My change doesn't affect this.

That's why it has TagName property so you can specify what HTML element you would like it to represent / render as. << ok good. My change doesn't affect this. So we both agree that setTagName should nearly always make getTagName return this new value.

TWebControl on the other side doesn't provide this functionality<< Right. TWebControl doesn't have setTagName.

  • you'd need to subclass it in order to represent any other element than a 'span' with in. << Right. TWebControl has getTagName from sub-classes to specify its own needed TagName.

The difference between THtmlElement and a TWebControl-derivant with an overriden getTagName() method is that for the latter you'd need to define a new class, whereas with the former you don't need to write any code, just place it in the template. << Right. But you can't isolate a within a skin without a skinid. Requiring a SkinId to do basic Skinning via the theme is not the experience i want developers to have within PRADO.

That's the sole reason why THtmlElement is there - to save the need to define and write a subclass.<< Except if you wanted a different default tag.... then you need to reimplement both get and setTagName.

Sorry, but with every new post you just make it more clearer that you obviously don't really know Prado and don't get a grasp on most of the concepts employed in it, or why things are done the way they are. Under these circumstances I have to press even harder for you not committing any serious changes to the repository without asking for comments first.<< Your personal attacks are not a reason for me not to continuing to commit intelligently and carefully thought out changes.

"Now it is being used by PRADO, for the added classes it's not bloat, and It's backward compatible. problem solved." No, you didn't solved any of the problems I pointed out, you just fled into total ignorance. You extension is still a bloat and completely pointless for the base THtmlElement class.<< I believe your too limited in your thinking of how people could use this new THtmlElement. If TWebControl has getTagName from sub-classes to specify their own needed TagName, what did THtmlElement have that was equivalent but in context of mutability? * I'm stating that the equivalent of TWebControl->getTagName is THtmlElement- getDefaultTagName in regards to subclasses due to the mutability built into it years ago. * If TWebControl subclasses can specify their TagNames, why can't THtmlElements (but still maintain built in mutability)? Adding another class to duplicate an existing class is bloat, and that I will not do.

Also sorry but your new controls make no sense at all. You added no functionality in those controls, rendering them completely pointless to have them as separate control classes. For ex. is completely pointless because you could achieve the same result with . (which would be just as suitable for skinning too).<< No. This is actually incorrect. These are not functionally equivalent. To re-iterate, would be the same as <com:THtmlElement TagName="h6" SkinId="SomeHeader6Skin" /> or using theming and as I have said, I don't want to require programmers to use the skinId to get changeable tags. People don't have a clue what THtmlElement is or can do but if you say THeader1, anyone that knows basic html will get it. The PRADO audience is not just high powered developers like you and me.

If you add THeaderX you could just as good add TRedDiv, TBlueDiv which have the Style attribute set to "color: red" and "color: blue" by default. << That is actually incorrect. the style color is an attribute. h1-6 are tags, not attributes. Making attributes into tags? lol. Do you really think of me as being that stupid?

And don't even stop there - add TRedDivWithFontSizeOf12AndAlignedRight too, because someone might need a div with a font-size of 12 and aligned right somewhere in their templates, in which case your new supercontrol could come just handy for them. Right?<< You should make a ticket for that. I'll NOP it personally. You are a funny guy! :D

--- My comments ---- Please realize that the more simple a control is the easier it is to do sophisticated theming. This is where I have to teach PRADO some new tricks in it's theming engine and TWebControls. What can i say? I like to separate my look and feel as much as possible from the html structure and the code. These changes are to support the separation of look/feel, layout, and code. PRADO does an amazing job of unlinking the page and php. But I think it can be taken a step further and the theme can define totally the look and feel without having to rely on any (or much ;) html in the page. If this isn't modularity incarnated I don't know what is.

Just because you don't see value in a code change doesn't mean others don't.

if you didn't catch that: * I'm stating that the equivalent of TWebControl->getTagName is THtmlElement->getDefaultTagName in regards to subclasses due to the mutability built into it years ago. * If TWebControl subclasses can specify their TagNames, why can't THtmlElements (but still maintain built in mutability)? Adding another class to duplicate an existing class is bloat, and that I will not do.

Lastly, I have no idea why you are taking 10 lines of code so personally. I've thought and thought and thought about the THtmlElement. I've thought about it way more than i want to. With that last paragraph with the ***, I have made up my mind. These changes are really quite reasonable and reasoned. Having gone through your arguments and not found any real substance in regards to THtmlElement but finding that you believe me to be in some way stupid, I'm not going to continue debating this with you. Unless another PRADO developer wants to change that code, my job is done.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 17, 2010 23:49:32

"I believe your too limited in your thinking of how people could use this new THtmlElement. " Sorry, but it's the other way round. I have no problem comprehending your intentions (as if you'd have read my comments carefully you'd have realized that I already knew you were to go create header controls based on your messed-up THtmlElement class before you actually commited those to the repository).

On the other see you obviously fail to see that there's a would out there outside of your mind too, where other people have other priorities for their applications built on Prado, and have no need and use for your extension. Now that wouldn't be a problem if you'd just add your extensions in a way that wouldn't degrade performance for them too (which as I explained to you numerous times could be done easily, by having your own intermediary class - this way other could still use the unneccessarily bloated THtmlElement class, and you could use your TMutatingHtmlElement whenever needed).

But no, you obviously think you're the only one using Prado, you own it, and you do with it whatever you want, disregarding implications for other. Sorry, but you're wrong. Very much so.

"Adding another class to duplicate an existing class is bloat, and that I will not do." Adding code to a base class (instead of doing that in a derived class) which most likely won't ever be used by anybody else except you is a bloat. And you just did that.

"That is actually incorrect. the style color is an attribute. h1-6 are tags, not attributes." Wrong. In your approach h1-h6 are merely attributes to the THtmlElement object. Hello, we're talking Prado here, not HTML!

"No. This is actually incorrect. These are not functionally equivalent. To re- iterate, would be the same as <com:THtmlElement TagName="h6" SkinId="SomeHeader6Skin" /> " Wrong. The only difference is: your theming system would have to be able to filter out targets based on attribute values too, instead of just class names. This would add far more flexibility to the theming/skinning system anyway, without adding all the bloat if you don't want to use theming at all.

Also, it's a mystery to me why you need to define that getDefaultTagName() method at all instead of just setting the TagName in the constructor or onInit method to the desired value (h1-h6, whatever)? Can you explain that to me? Because that would eliminate the need for the superflous getDefaultTagName() method and you would have to write roughly the same amount of code in your obviously also ill-fated THtmlElement-derived classes.

"Just because you don't see value in a code change doesn't mean others don't." I see the value in it for you, but I also see the negative effects of it for others. That you either are don't see or you are simply ignoring it, because it's not a concern to you.

"Lastly, I have no idea why you are taking 10 lines of code so personally. " God, only if you'd switch off write-only mode and read my comments thorough before replying to them. I already and explicitly explained to you multiple posts ago, that it's not about the 10 lines of code, but the basic principle. Now you're adding 10 lines of bloat here, and 10 lines of bloat there, and in a month you made Prado 10 times slower than it was and have transformed it into something that works perfectly well for the purposes of your own CMS, but has become a nightmare for anybody else to use. That's what I don't want to happen, that's why I have to object already because of this seemingly minor change.

You prove with every new post and commit that I'm right about and that if you were to given free hand over Prado you most likely would create a monster out of it in no time.

"Your personal attacks are not a reason for me not to continuing to commit intelligently and carefully thought out changes." Actually, you'd first have to start doing that in order to be able to continue with it. So far you failed to do so.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 00:54:17

"No. This is actually incorrect. These are not functionally equivalent. To re- iterate, would be the same as <com:THtmlElement TagName="h6" SkinId="SomeHeader6Skin" /> " Wrong.<<<

You proved my point. Thank you. Please read up on the TThemeManager and how it works before getting into a debate about how it works.

Bloat is bad, true. This is not bloat. AGAIN, as i have said so many times... please be patient.

Funny you mention that you knew I was going to do that.... I was telling you over and over and over that that was coming.

If you don't want new features in prado you should stick with the 3.1 branch because that's what it sounds like you want.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 01:05:14

Here is what I am all about: Small tweaks that open up entire new avenues of functions, features, and development that you as a user of PRADO can then capitalize upon. I don't consider this bloat. My websites run 750,000 hits a day. I know how important it is to keep things lean, mean, and fast.

Your concerns that PRADO will turn into something that others cannot use are unfounded. 3.2 will be as backward compatible as possible. If you find the current system usable, things will be fine.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 01:05:25

"You proved my point. " On the contrary. I just pointed you to a better solution of the problem, which would avoid all bloat associated with your solution and provide an even better way to re- skin application than what's possible with your solution in the first place. I'm sorry you don't even get a grasp on how much more flexible such an extended theme manager would be in skinning, which would not only eliminate the need for any of your THtmlElement subclasses, but would also make those Prado application/pages skinnable - without the modification of the original sources/templates and without the need of SkinIDs - which were never designed with skinning in mind the first place.

"Funny you mention that you knew I was going to do that.... I was telling you over and over and over that that was coming." What you were telling that something was coming - you didn't tell what, but it was just too easy to figure out. Probably because I think two steps in advance of you and already point out troubles you don't even see coming yet.

"If you don't want new features in prado you should stick with the 3.1 branch because that's what it sounds like you want." You obviously have problems understanding plain english. I didn't oppose new features - I only opposed bloat and poorly designed "improvements" from you. Hope you can comprehend the difference between the two concepts.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 01:19:40

"Your concerns that PRADO will turn into something that others cannot use are unfounded." I wasn't concerned that "PRADO will turn into something that others cannot use". Read my posts again if you believe I did say so (because I didn't).

"My websites run 750,000 hits a day." So are we again at the "whose is bigger" competition? I'm asking because my sites make >1 million hits a day. Still just as with my experience with OOP - which is larger than yours as I have +5 years on you - I'm not telling you that you should consider my opionion because I have sites like this or that. Instead I explained to you in detail and in technical terms and reasons why your design solution is ill- fated, and why would a different approach be far more effective for both of us, completely eliminating the negative sides of your design. Obviously, to no avail.

"Here is what I am all about: Small tweaks that open up entire new avenues of functions, features, and development that you as a user of PRADO can then capitalize upon. I don't consider this bloat."
It is bloat if use of that extra feature is not optional and if it consumes resources even when not needed and uses. And exactly that was/is the case both with your event-tunneling approach and with this default tagname madness.

"3.2 will be as backward compatible as possible. " This is about the 10th that you start talking about backward-compatibility and telling me that I shouldn't be concerned about. Now the problem with that is: I was never worried and never did complain about backwards compatibility regarding this issue. Rather, what I was and am complaining about is bloat and degraded performance due to poor design. So you can tell me for the 1000th time that your modifications are backwards-compatible (which I never debated), it still won't make my points about bloat and bad design invalid. Get it?

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 01:34:12

Your suggested changes to the TThemeManager should be in its own Issue.

Again, read up on the TThemeManager. Until you understand how theming works, and can discuss it properly, I will not continue this conversation where all you do is insult me.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 01:36:25

I'm also still waiting for your explanation and reply to my question about why you need the getDefaultTag() method in the first place, instead of just overwriting the default value of the TagName property in the constructors of your derived classes from 'span' to 'h1'-'h6'?

You know like:

class THeader4 extends THtmlElement {

public function __construct()
{
         parent:__construct();
         $this->setTagName('h4');
}

}

It's roughly the same amount of code for the derived classes as now, still, you'd not need to introduce a new method (::getDefaultTagName) which is completely useless in THtmlElement itself, and you wouldn't need to mess in its getTagName() method. Which is a bloat (at least in literal sense) in itself too, because instead of:

if($this->_tagName !== null) return $this->_tagName;
$this->_tagName = $this->getDefaultTag();
return $this->_tagName;

you could have just written

return ($this->_tagName !== null) ? $this->_tagName : ($this->_tagName = $this-

$this->getDefaultTag());

which is obviously far more compact. Not that the latter would really matter, because what I was complaining about was a bad design decision, not about not using some PHP über-hacks.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 01:42:05

"Your suggested changes to the TThemeManager should be in its own Issue. Until you understand how theming works, and can discuss it properly" You first acknowledge that I just told you how to improve on it, and then you continue on with you telling me to look it up how it works. How could I propose improvements if I wouldn't know how it works and what it's limitations are? Makes your argument sense to you? Because it doesn't to me.

"I will not continue this conversation where all you do is insult me." Actually, I think you rather have problems defending your position in technical terms. At least I see no real arguments from you why and where I would be wrong with what I'm saying. Your only "arguments" against my propositions are how stupid I supposedly am. I at least tell you the "why"s too. :)

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 01:43:30

Beginning programmers tend to forget to add parent::__construct().

Also having a defaultTagName is the same way TWebControl does TagName with its subclasses. It's basically the same OOP solution pattern to the same problem only this adds mutability.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 02:08:03

"Beginning programmers tend to forget to add parent::__construct()." Yeah. Beginning programmers also tend to forget to close down lines with ";" or to encapsulate the condition in "if" constructs into parenthesises. That's not something a framework designer should care about.

"Also having a defaultTagName is the same way TWebControl does TagName with its subclasses. It's basically the same OOP solution pattern to the same problem only this adds mutability." Wrong. Mutability is already there just because THtmlElement's TagName property is write-able. You can change of any THtmlElement - or derived - object's TagName based on skin or whatever, even without having any getDefaultTagName() methods and messing around in the getTagName() method of THtmlElement. Even in the current theme/skin implementation the properties specified in the .skin file override those specified in the template, so to for ex. change all H2 elements to DIVs you simply have to add the line

to your skin file. And with the advanced theming system I'm proposing you wouldn't even need to declared THeaderX and all the other classes just specify something like this

/com:THtmlElement Of course the syntax of the filter expressions or the way they're specified could be changed, I just made that up as an example. There could be also multiple matching modes (like allow for partial/substring matches, ranges, or whatever, the ability check for parent's attributes or ownership, etc), and this would allow far more flexible theming/skinning without the need of skinids or derived classes (like THeader1, THeader2, etc) just for the sake of being able to apply a separate skin definition on them (because right now the theming system can only filter based on class name, but not attributes).
ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 06:59:52

And with the advanced theming system I'm proposing you wouldn't even need to declared THeaderX and all<< As I stated, you should propose your theme enhancements in another ticket.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 07:01:36

Just to be clear. I'm agreeing to disagree on this issue.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 08:45:47

Also... I have to say thank you for pushing me. I don't think i could have come up with the getIsMutated method without your help, at least not this early in my thinking of THtmlElement. It could be a very useful function going forward! Cool :D I could sense that there was a reason I was entertaining this debate.

ctrlaltca commented 11 years ago

From google...@pcforum.hu on April 18, 2010 14:17:58

"As I stated, you should propose your theme enhancements in another ticket." You introduced your modification with the sole and only purpose (acknowledged by you) to solve a problem with theming. Yet you want me to open another ticket for a solution to the very same problem, which, however would have a lot on your solution. Makes sense. Not.

"Just to be clear. I'm agreeing to disagree on this issue." That was obvious all the way through. The only problem is that you presented no arguments at all to support your approach and your opposal to my solution, while I already presented you not only with a lot of technical counterarguments against your approach, but also came up with a far better solution even to your basic problem.

"I don't think i could have come up with the getIsMutated method without your help, at least not this early in my thinking of THtmlElement. " Yeah, sounds about right as the basic problem with your doings. You code first and think only afterwards. If ever. You should think more first, and only code and commit afterwards.

ctrlaltca commented 11 years ago

From javalizard@gmail.com on April 18, 2010 15:56:45

dude. i like your idea. That's why I'm suggesting you put a ticket in! I think it is worth debating if it has some benefit to be able to filter by other properties besides just the SkinId. My solution and your solution actually solve slightly different problems. I do not have malevolence toward you, so i don't know why you continue to personally attack me.

you presented no arguments<< Um... as far as I know, I didn't delete my comments with my argument. I'm calling bs on that. I'll refer people to do a search on this page for * and you'll see my arguments. I ask that you refrain from trying to put words in my mouth...
or in this case, no words. By the way, if you want to get all mind-gamey, I marked my comments with those
* just in case you said exactly what i quoted you as saying so I could refer back to them. So, please. Everyone's "two steps ahead" are sometimes faulty, yours and mine. I'm not saying all your ideas are bad. I rather like many of them. I'm just saying that we are all wrong sometimes. Myself included. I've already demonstrated my ability to be adaptable, flexible, admit that an idea of mine wasn't the best, evaluate quickly better alternatives, and implement them before they get embedded. (eg. TPageService::onPrePageRun event). The best an entrepreneur and/or developer can do is recognize her or his mistakes as early as possible and correct them as quickly and efficiently as possible.

Have you worked with the Agile Software Development Methodology? It's an iterative development process. I really try to be as thorough as possible on the first pass but, you know, things change. That's a good thing. Building the future is a very hard thing to do right and requires feedback from everyone. Can you place comments on code in the repository?