Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it an antipattern? #50

Closed
alexeyraspopov opened this issue Mar 27, 2012 · 3 comments
Closed

Is it an antipattern? #50

alexeyraspopov opened this issue Mar 27, 2012 · 3 comments

Comments

@alexeyraspopov
Copy link

Hi guys. I've watched all of yours topics about patterns and antipatterns. Nice job! But, I have comments and questions for some topics. I think, some of your patterns is not good. So let's check.

regular-expression-literal.html
Using RegExp constructor can't be antipattern because sometimes we need to create regular expressions dynamically with some input data. For example:

//I know what this example is trivial and not good but it's only for example
function hasClass(element, className){
    var regular = new RegExp('(^|\\s)' + className + '(\\s|$)', 'g');
    return regular.test(element.className);
}

enforcing-new.html
Let's be more logical. If I write functionName() I want to call some function. If I write new FunctionName() I want to create some object. Using your pattern I can create new object even when I call function. But for what? Because we can save 4 symbols and it can minimize file size? Huh, it make me laugh. I think logic of code is more important than file size.

avoiding-implied-typecasting.html
I think, one of antipattern in JavaScript is comparison with true/false/null/undefined. Always we can avoid this comparison using next construction:

var zero = false, n = null;
if(!zero){    //instead of zero === false
}
if(!some){    //instead of some === undefined or typeof some === 'undefined'
}
if(!n){    //instead of n === null
}

for-loops.html
http://jsperf.com/for-loop-caching As you can see there is no difference. And when we cache array size we create new variable. I think it's not useful variable.

for-in-loops.html
For the last example. Why I should save link to Object.prototype.hasOwnProperty when all objects are have this method?

function-declarations.html
This topic is real antipattern. Why I can't use simple function declaration?
The difference between function someFunc(){} and var someFunc = function(){} is the moment when they are created. Simple function declaration will create function before script executing. Declaration function like new variable will create function when script will be working. And all of this functions will be local. So which variant is antipattern? But, simple function declaration can be used only in global scope or first IIFE. Otherwise local function will be created in advance even if you don't call the desired function.

conditionals.html
I think, construction bool && boo() is not logical because it's not expression, it's condition. And condition can't be used like expression.

if (({foo:1, bar:1})[type]) {}

Create new object for one condition? WTF?

revelation.html and module.html
Why you use comparison object with "[object Array]" to indicate arrays when you can use object instanceof Array or object.constructor === Array.

klass.html
Classes in JavaScript... Again...
Why people can't understand prototypal-based programming and use it instead of this ugly classes? Why all prefer to use classical OOP?
First of all, classes are data types. In JavaScript we work with objects and we have only some primitive types like: number, string, boolean, function. We can't create our personal type. We can create some objects with the same properties.
Secondly, classes unites data and method to work with this data. Constructors in JavaScript do this too. Classical OOP was complicated by a variety of ways to create a class. And, when we create arcitecture of our application one of the main problem can be "Oh, in this class I can use virtual method. Or I should use composite pattern? Hmm...."
"There should be one — and preferably only one — obvious way to do it." (The Zen of Python)
In JavaScript we have only one way to describe objects. We create a function which will be a constructor. Then we can add some properties to prototype object (which placed in our constructor). All properties in prototype are common to all instances of our constructor. That's all. It's realy easy.

I hope you will read all this comments and understand my engilsh :)

Best regards,
Alexey Raspopov

@maksimr
Copy link

maksimr commented Apr 6, 2012

###enforcing-new.html

I do not think that in this example, the main thrust is to reduce the number of input characters. So you do protection inside the function constructor. Because if someone accidentally calls her without a new one, it can create global variables or fall with an error in strict mode.

###avoiding-implied-typecasting.html

I agree with you that in most cases this will be the construction that you described, but as I understand it, this example suggests that it is necessary to avoid an automatic type cast, as this can lead to errors as the beginners and experienced developers.

p.s. In you example you get error because some is not defined ;)

###for-loops.html
http://jsperf.com/for-loop-dom (this is a very rough test =) )
I think this example is designed for the most part to work with the tree of the document, as the re-conversion of length collections elements rather expensive operation (such as collections are alive). And if no difference I prefer cache variant.

###for-in-loops.html

  1. Unfortunately it is not(Personally came across this). Some DOM object does not has this property(or work incorrect).
  2. When you call someObject.hasOwnProperty It will be a look at the whole prototype chain until it reaches Object.prototype, additional time spent.
  3. It flows from 2. The object has this property can override anyone else.

###function-declarations.html

Well, as you may have noticed not only a function declaration that is determined before the execution of code, so more and overwrites all that already has that name, and even other function declaration (and it is not corrected during the execution of the function). And since they are determined to run the code it is not always and not for everyone obviously, too much is not always a good trick, especially if you are in command. Oh, and lastly, such a function can only be defined in the body of a function or program level, which is pretty much reduces the area of ​​their ads.

###conditionals.html

&& and || are logical operators, but some times you can use them how something like conditional expressions because they return value of operand.

###revelation.html and module.html

var fakeArray, MakeFakeArray = function () {};
MakeFakeArray.prototype = Array.prototype;

fakeArray = new MakeFakeArray();

(fakeArray instanceof Array); //true
({}).toString.call(fakeArray);//"[object Object]"

p.s. Надеюсь я правильно понял твои замечания и сам что-нибудь полезное ответил. =)

Maksim

@Alexey-Raspopov
Copy link

###enforcing-new.html

99% of code's errors is sitting behind the monitor, right? :)

###avoiding-implied-typecasting.html

there are no strict mode in my example :P

###for-loops.html

Thanks for your test, I didn't know about this.

###function-declarations.html

If you work in team you can DISCUSS that moments and problems what you describe will disappear.

###revelation.html and module.html

To your example and to all your comments:

I almost agree with all your comments, but all problems what your have described can be avoid if you create standarts to you and your team, if you provide 100% code review for all commits and if you discuss code with your teammates

@nmartynenko
Copy link

jQuery data anti-pattern

I suppose that this is not anti-pattern due $.data and $.fn.data may be used in different cases:

<div id="entryHolder" data-entry-id="1">
...content
</div>
var holder = document.getElementById("entryHolder");

alert($.data(holder, "entryId")); //shows undefined

alert($(holder).data("entryId")); //shows 1

//BUT!
alert($.data(holder, "entryId")); //shows 1
  • $.fn.data can be used in chainings, $.data - not
//legal
$(".selector")
   .data("test", "val") //returns jQuery object
   .slideUp()

//illegal
$.data(elem, "test", "val") //returns value
    .slideUp(); //throws exception
  • $.fn.data works with jQuery objects, meanwhile $.data works with DOM elements. Therefore if you had DOM element it's better using of $.data .
//bad practice
$(domElem).data("some", "value");

//preffered
$.data(domElem, "some", "value");

NOTE: It doesn't fluent to chainings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants