Code Guidelines
in [JavaScript]
|
Prev: Dynamic iframe caching problem workaround needed for Firefox
Next: JavaScript code mangler / scrambler / ... khm, more than obfuscator... :)
From: Dmitry A. Soshnikov on 1 Jan 2010 14:06 On Dec 31 2009, 11:23 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: [snip] Continue to answer from this point (as I've mentioned, I didn't see that half of your answer because of "read more >> " link). > > >> Everything has > >> String.prototype, right? > > > Absolutely right. > > >> Well, now if you modify that, then everything > >> has that modification. > > > Of cause not. "Everything" will have that modification when that > > "everything" will *use* that modification. And there's no difference > > between 'string'.capitalize() vs. VeryOwnStringNamespace.capitalize() > > - dependency is equal. > > By "use", I believe you mean to describe code that is accessing the > `capitalize` method, and not code that exists where the script that > defines `capitalize` exists. > Yep, because (and I've already repeated not once) and `String.prototype.capitalize' and `M.capitalize' can be defined in the same exactly place (file) and used in the same exactly places. > Either way, I don't think it matters much. Once the API is published, > you don't get to decide who calls what. You can say: "I defined this > property, but don't use it," and if there is no value in using it, then > probably nobody will use it. If there is some value in using that > feature, then it has a better chance of being used. How would you know > who is using `capitalize`? > > Either way, the method is globally accessible. > Yeah, and that's exactly I'm talking about, when telling that there's no difference. > >>> Moreover, I tell, people which use libraries think vice versa - they > >>> think: "we don't not modify String.prototype as we're using 3rd-party > >>> frameword, which can put tomorrow (in the next version) `capitalize' > >>> method into it. So let's make our own namespace such as > >>> OutSuperDuperNamespace.string and put `capitalize' there." And > >>> tomorrow, OMG, that framework provides the same > >>> `OutSuperDuperNamespace' and even `OutSuperDuperNamespace.string' > >>> breaking down all the project. So, that's not the reason. > >> First off, who creates a namespace like "OutSuperDuperNamespace". Then, > >> of those people, who then goes on to include *another* library that also > >> the same (fruity) namespace? > > > The main idea (and understand that) is to show that you don't exactly > > know what will some library provide "tomorrow". So your own "APE.dom" > > may be overridden by all of that libs you'll use. > > I think we are arguing the same point. > Yeah, I see that too. > That is, you don't know what may be defined tomorrow. > > That public interface, is a property of global object, and so it could > be replaced by a simple assignment. > > Anyone using that namespace could be expected to not redeclare that > namespace and to not use another piece of code that does. > > YUI's YAHOO.namespace method fails on this account. > > YAHOO.namespace fails because it adds user-defined properties to the > YAHOO object. Calling YAHOO.namespace("example") creates YAHOO.example, > if it does not exist alreay. YUI's namespace strategy makes it easy for > conflits to occur. Consider a client using YUI that wants to define > YAHOO.touchevent namespace. Will a future release of YUI have its own > YAHOO.touchevent object? YAHOO has no way to be certain that a client of > YAHOO had not followed YUI's advice and used > YAHOO.namespace("touchevent"). If YUI decides to use YAHOO.touchevent = > {}, then the namespace will be obliterated. > That's it. > >> A sensible developer would organize the coe into modules. > > > I know what modules are, be sure. `String.prototype' - is the kind of > > a module itself. And if you are sure what you are doing, it's > > absolutely not a bad practice to put `capitalize' into it. > > String.prototype is a built-in. If it is to be called a "module", then > it is a built-in module. If you've modified it, then it is a built-in > module with a mix of user-defined properties. > Yeah, and I repeat, the main goal is about useful and elegant code such as 'string'.capitalize() where `.capitalize' is put *logicaly* there where it's place for it - in the strings "module/class". And that's all about mostly when there's no some issues as 3rd-party libs using. > If user-defined properties are defined, then where should these > definitions occur? > > If the definition of user-defined additions to String.prototype is > defined in a author file that is defining another module, then the > author is defining two things in that file instead of one. So those > modifications should not occur in same author file as another module's code. > > By "author file" I mean the file the author edits, not necessarily the > same file the http client receives. > > The author file should be defining one thing only. > I don't see a big issue. What's the prob - user can define `String.prototype's extension in separate file - then will be "one file for one thing", where "thing" is "patching built-in with useful methods for to write more useful code instead of Ext.util.Format.something". > >> Obviously > >> "DOM" (in any case) would be likely to conflict on global level. I use, > >> for example, APE.dom. > > > That's good, but you understand that if you'll using dozen of 3rd- > > party libs combined, your "APE.dom" theoretically can just gone in one > > moment - when all the libs will provide the same names and structure. > > Do you see the difference in this case from augmenting built-ins for > > own goals (when you know and understand what are you doing)? > > The only user-defined identifier that is ReadOnly is the Identfier for > FunctionExpression. And that doesn't work in JScript, as you probably know. > Yeah, thanks, be sure, I know (although, it's not about the topic). > By that fact, you are are right; any one library can replace APE.dom. > > A conflict can occur when *one* external module or library also modifies > the property String.prototype.capitalize. > Yes, sure. Have you noted that we're talking mostly about issues with 3rd-party libs? Meanwhile, we've put this fact into the issues when augmenting can be the issue in case when some lib will do the same. But what I'm talking about - is mostly regardless all mentioned issues - that it's not a bad practice, when you understand why you do so. This will help to write more useful and elegant code. But sure, when to augment with using 3rd-party libs - that's already "for own fear and risk". > The point is to not do that; to not modify objects you don't own. > > Instead, I am suggesting that methods exist as part of units or modules > and with an easily identifiable role. > > This can be achieved by defining one global namespace and hanging > properties off that. I understand that, and this structure is sure useful, but when describing some methods that related to the strings - for what additional module is needed? That "M" (and to write in one place 'string'.trim() and in other M.trimLeft(string))? For to know that that's your extenstion but not a built-in? For do not make conflict with the next Standard built-ins? Sure all of this clear, but when the issue will be, then will find the solution. But for now we can write more elegant and useful (and logically more correct in my opinion) code such as 'string'.trimLeft(). > > If it is OK to modify String.prototype, is it okay for everyone to do > that, or just an internal organization? > > Is it ok to modify objects you don't own in general? If so, are there > exceptions to that rule? > Good question. I think so: for exactly built-ins can be exceptions as built-ins come from the beggining and every user (you or 3rd-library) can do with them anything they want. So again, if you don't use some 3rd-party library, it can be useful to augment some built-in for to write more useful and elegant code. If you want to touch some object of 3rd-party (which you also can do - to patch it) - you already do it for your own fear and risk. And here can be useful to touch the properties which should appear in the next releases (e.g. if you're using v.2 and know that in v.3 will be a method doThat() - so you add your own implementations of doThat()). I understand, that in case of built-ins can be exactly the same, but in augmenting of both: built-ins or some 3rd-party library you may also add your own patches regardless new versions - that which just useful for you. > [...] > > > > >>> But if you'll write - "Augmenting built-ins - is a bad practice" - > >>> that will be categorical and wrong, and I can statement, that person > >>> which spread that not completely understand the goal of dynamic > >>> languages (I do not mean exactly you, I'm telling abstractly now). > >> I'm not completely sure what that means. > > > That just means - that ideologically it's normal to augment built-ins > > if language is constructed so and if this fact is in it's ideology. > > The language allowing something does not make that thing ideal. The > language makes assignment to identifier end up with globals. How many > times have you seen a missing - var - statement. > > for(i - 0; i < 10; i++) > > ? > Well, yeah, and that's why I've told repeatedly that user should understand what he is doing. If he does, he can use the system in a full power, if not - can catch every time errors such as example above. > One useful way to modify built-ins is to add the standard feature for > implementations that have not yet implemented it, or have implemented it > wrong, but preceeding that with a feature test, e.g. > if(!String.prototype.trim) ... . > Yep, sure, I've told above about that. But also, you may add your own additional functionality regardless new versions (maybe such functionality which will never be added by Standard or by some lib). > If the rule "don't modify objects you don't own", is followed, and the > only exception to that rule is to add a global property, then conflicts > should not occur. > > > So, I don't propagate everyone to augment built-ins (I think you think > > so about my position, so I'm telling you - nope). My position is to be > > fair for augmenting built-ins (which means, it against the categorical > > statement "Augmenting built-ins - is a bad practice"). To augment or > > not in this case - everyone choose - understanding all the issues. > > I got that. I don't see a good reason for modifying objects you don't > own. I see a few downsides:- > 1) increases potential conflict with > * new standard > * new version of 3rd party library > * code defined somewhere else in the organization > Yep, I've told about the issues. > 2) Method presence doesn't provide indication where that method is > likely to be defined. You mean if "M.foo.bar" means directory "M", subdirectory/file/module "foo", method "bar" - you know where it is. And with 'string'.yourOwnMethod() you don't know where to search the definition of the `yourOwnMethod'? I don't see that this a big issue. As variant you can provide `built-ins-extensions.js' and do it there (or as usually, some `core.js' or sort of and there sub routine for initialize extensions of the built-ins). So again, I'd like to say about my position: if do not touch all the issues (main of which are: (a) Augmenting of the Object.prototype and (b) using 3rd-party libs) - it can be useful in your own projects, and you can write more clear, useful and elegant code (and that's all this about, 'cause from functionality point of view, repeat, there's no any difference where to put this code). But sure, if you do so with regarding the described issues (especially using 3rd-party libs) - you always do this for your own fear and risk. /ds
From: Dmitry A. Soshnikov on 1 Jan 2010 14:37 On Jan 1, 9:34 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: > Dmitry A. Soshnikov wrote: > > On Jan 1, 8:43 pm, "Dmitry A. Soshnikov" <dmitry.soshni...(a)gmail.com> > > wrote: > > > [snip] > > >>>>> Everything has > >>>>> String.prototype, right? > >>>> Absolutely right. > >>>>> Well, now if you modify that, then everything > >>>>> has that modification. > >>>> Of cause > >>> ... > >> Ok, I've "appreciated" this demagogy in this part of your answer ;) > > > Sorry, Garrett I didn't see a half of your answer because of "read > > more >>" link (I didn't see it before here), so I thought you write > > only "Of cause and ...". I'll read the full your answer and will add > > some answer to mine previous. It wasn't your demagogy in this place, > > apologize. > > You're using Google Groups, then, I take it. > -- > Garrett > comp.lang.javascript FAQ:http://jibbering.com/faq/ Yes I use Google Groups to see/answer. And what do you? /ds
From: John G Harris on 1 Jan 2010 15:17 On Thu, 31 Dec 2009 at 23:42:02, in comp.lang.javascript, Dr J R Stockton wrote: >In comp.lang.javascript message <xtlkCULx43OLFwTX(a)J.A830F0FF37FB96852AD0 >8924D9443D28E23ED5CD>, Wed, 30 Dec 2009 16:22:09, John G Harris ><john(a)nospam.demon.co.uk> posted: >>On Wed, 23 Dec 2009 at 19:39:41, in comp.lang.javascript, Dr J R >>Stockton wrote: >>>In comp.lang.javascript message <+KuB1xEneKMLFwQf(a)J.A830F0FF37FB96852AD0 >>>8924D9443D28E23ED5CD>, Tue, 22 Dec 2009 11:04:07, John G Harris >>><john(a)nospam.demon.co.uk> posted: >>>>On Mon, 21 Dec 2009 at 15:52:55, in comp.lang.javascript, Garrett Smith >>>>wrote: >>>> >>>> <snip> >>>>>Formatting: >>>>> * Tabs used instead of spaces. >>>> >>>>Spaces are preferred, I hope. >>> >>>Some like larger indents. And a good viewing system can be set to make >>>a tab equivalent to two or three spaces. On the Web, however, tabs will >>>normally be worth up to 8 spaces, and should not be used as the indent >>>unit since most readers will find that too big. >>> >>>AFAICS, however, tabs are fine for comment and in code strings and in >>>simple tables. >> >>Anyone who has set up their tab stops to view tables are not going to be >>happy setting up 30 or so tabs to view a bit of javascript. Tabs are >>convenient for writers but a nuisance for readers. >> >>Two spaces are necessary and sufficient for the indent interval. >> >>We are assuming, of course, that sensible people use a fixed-pitch font. >>Anyone who uses a variable-pitch font deserves to be confused. > >I think that you need to read what I wrote more slowly. > >The proportion of people whose Web browsers and/or newsreaders are >routinely set up to have tab stops at other than 8N+1 must be >negligible. The number of people using a browser or news reader to edit code is even more negligible. >Remember : > the most important reader must be the writer, No, the most important reader is the poor unfortunate who has the job of updating or correcting the code, especially when they are not the original author. > the code as written is not necessarily the code as transmitted. John -- John Harris
From: Garrett Smith on 1 Jan 2010 19:13 Dmitry A. Soshnikov wrote: > On Dec 31 2009, 11:23 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> > wrote: >> Dmitry A. Soshnikov wrote: >>> On Dec 30, 4:04 am, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>> Dmitry A. Soshnikov wrote: >>>>> On Dec 28, 9:25 am, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>>>> Dmitry A. Soshnikov wrote: >>>>>>> On Dec 27, 10:45 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>>>>> [snip] >>>>>>>> A public library that modifies >>>>>>>> the built-ins can't really be trusted to work with other code. [snip] > Let me again to mention the real dependency (regardless some "build > tools") - the dependency *is*, when some code uses that `.capitalize' > method, regardless how was it described. > > If to take in mind you "build tool", I think augmenting of some built- > in object you'd like to put in some "global/general/built-in/common > -.js" which includes to *every* file. Meanwhile, "m.js" with own > namespace `M' and method `M.capitalize' will be included only in > needed files, right? Do I understand you correctly? Is this you want > to tell me? If so, let me again to mention about real dependency - > when code will *use* concrete and exact `.capitalize' method, > regardless *including* by some "build tool". Moreover, nothing > prevents to place `String.prototype.capitalize' also "m.js". > >>>> Putting the captilize function in a separate separates the concern so >>>> that only as many modules as necessary depend on that module. >>> Nope, how can't you see - that the dependency is equal in case you're >>> describing. >> A module that adds a property to something it doesn't own introduces >> something globally accessible that unrelated to the module itself. It is >> doing more than it should. It is not really modular. >> > > First, not exactly "globally". The thing that the module adds is globally accessible anywhere the module is used. Second, it's not the module who puts > "something" into the "something". That *you* augment the built-in > module with new functionality to make more readable and useful code (I > remind, `String.prototype' is a module itself - so you put new > functionality into it. But not "some module is doing more than it > should"). > I did not understand that. [...] >> // No good if it is defined (in ES6, etc) >> if(!String.prototype.startsWith) { >> >> } >> >> An alternative is the code could instead redefine >> String.prototype.startsWith to String.prototype.myVeryOwnStartsWith. >> >> But then we are getting to the point where the method name is trying >> hard to have an identifier that identifies it as being something added >> not built in. >> >> >> > > Here you're describing something concrete and already regardless topic > (it's not so interesting how `capitalize' is implemented in ExtJS or > in Prototype.js, this method was taken only for an abstract example of > augmenting) - is it a good or bad practice to augment built-ins. So I > let myself to answer shortly - even regardless is a "good" or "bad" > candidate for "some next Standard", it's absolutely not a bad practice > from my point of view - if you understand what are you doing. That > will let to write more clear and useful code. > The reason for looking at a concrete example was to look at why one might modify String.prototype vs create own object. Modifying String.prototype has some not so good consequences. They are: (1) greater likelihood to conflict with something else than YourOwn.stringFormat.capitalize. (a) future version of ES, (b) other third party libs. (2) is less clear about where it is defined. Does `captitalize` have to do with strings *in general*, or does it have to do with the way your application *uses* strings? If so, then there is greater likelihood of conflict with a future ES version. Otherwise, if the method is *not* purely related to strings, then what is the justification for adding it to String.prototype? [...] > So, concluding this ('cause I don't wanna to make a useless dispute - > I see what see, I know what I know), some questions I have to you: > > 1. Do you understand that your *statement* about augmentations of > built-ins - is just your own (humble) opinion? (Actually, exactly the > same as mine. But in difference from yours, mine doesn't forbid, but > explain why it's good and when it can be the issue). > I don't see design decisions as amtters of personal preference, if that's what you mean. I see them as having consequences. The consequences of modifying objects you don't own outweigh any benefits to the coding style. > Also, I'd like to mention, that firstly your statement sounded as > "Don't modify objects you don't own". There you didn't talk about > "built-ins" bug about abstract "not own" objects. Although, in our > talk I found out that you mean also built-ins. For me, touching some > *other* objects that I don't own (but not a built-ins) - is also can > be useless practice (although, I don't use words "bad practice", > 'cause even regarding not to built-ins but to some other - it could be > useful to make some patch on some 3rd-party code, although, they can > do it themselves in new version of library). > Are you suggesting to modify a 3rd party method by shadowing it with a patch? That sort of reverse inheritance creates a dependency cycle. That can easily cause a problem where the "patch" causes a bug somewhere else. Format (other) TableRenderer (own) Lets say your TableRenderer depends on Format and Format.capitalize has a problem, so you replace it it your code with:- Format.capitalize = function() { /*...*/ }; And it works. Great so you patched it. Now it capitalizes those funny german characters just as the German customers want, and it ignores some wacky leading whitespace chars that somehow the user managed to paste into the text input. Fixed it. Good. Now for example, something else in Format, Format.captitalizeTrim (whatever) depends on capitalize. TableRenderer doesn't care; that captitalizeTrim code path never gets executed. When capitalizeTrim is called, it gets *your* patch, but only when TableRenderer is present on the page. When TableRenderer is not present, it does not get your patch. Say now that captitalizeTrim has problems with your patch, perhaps only occasionally. TableRenderer has dependency in Format, Format has dependency within TableRenderer. That dependency cycle creates a problem. Instead either: a) create your own capitalize routine, but not replace the existing. b) modify the source code directly and test it wherever it is being used. Obviously (a) is much easier and smaller in scope, better for a hotfix, but if the patch is safe and would fix (possibly hidden) bugs elsewhere, then it makes sense to fix the bug. You might also want to file a bug report, though don't expect an immediate fix with third libraries. I have filed bugs over 1 year ago that remain open. Of course I patched those bugs myself in the source code. > 2. I'd like to hear how exactly your position sounds. I've already > mentioned my position about this question: "I don't force anyone to > augment built-ins, I don't spread this idea, I'm just not agree with > the statement that augmenting of built-ins in dynamic language with > mutable objects - is a bad practice. Again - it's normal practice and > let to write more useful code against that long ugly as in ExtJS for > `.capitalize'". > Are there any good reasons for modifying the built-ins? > So whatever you write (for whom this "Code Guidelines"?) - will you > write it or not (that it's a "bad practice") - I don't mind. But if > some using this "Code Guidelines" will (or will try to) judge anyone > by that question (about augmenting of built-ins) - I'll tell again, > that that people don't understand why they are judging so and they > don't think with their own head. Also that will be a bad practice of > using such "Code Guidelines" for *novices*, which will think that > everything written there (in statement manner!) - is conclusive true. > But it's not so. > I'm writing the Code Guidelines for basis code reviews here. An example of modifying not own objects that we saw recently was where dojo adds a _listeners property to a function. The next document is Code Review Guidelines. We are going to have good code reviews here. -- Garrett comp.lang.javascript FAQ: http://jibbering.com/faq/
From: Garrett Smith on 1 Jan 2010 19:22
Garrett Smith wrote: > Dmitry A. Soshnikov wrote: >> On Dec 31 2009, 11:23 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> >> wrote: >>> Dmitry A. Soshnikov wrote: >>>> On Dec 30, 4:04 am, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>>> Dmitry A. Soshnikov wrote: >>>>>> On Dec 28, 9:25 am, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>>>>> Dmitry A. Soshnikov wrote: >>>>>>>> On Dec 27, 10:45 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: >>>>>>>> [snip] >>>>>>>>> A public library that modifies >>>>>>>>> the built-ins can't really be trusted to work with other code. > [snip] > [///] > > Are you suggesting to modify a 3rd party method by shadowing it with a > patch? > Correction: not "shadowning it with a patch" but "replacing the value where needed". [...] -- Garrett comp.lang.javascript FAQ: http://jibbering.com/faq/ |