9

I am using Webstorm and I wrote a React component, my code looks like:

async onDrop( banner, e ) {
    banner.classList.remove( 'dragover' );
    e.preventDefault();

    const file = e.dataTransfer.files[ 0 ], reader = new FileReader();
    const { dispatch } = this.props;

    const result = await this.readFile( file, reader );

    banner.style.background = `url( ${ result } ) no-repeat center`;
    dispatch( addLayer( file ) );

    return false;
}


@isImage( 0 )
readFile( file, reader ) {
    reader.readAsDataURL( file );

    return new Promise( function ( resolve, reject ) {
        reader.onload = ( event ) => resolve( event.target.result );
        reader.onerror = reject;
    } );
}


onDragOver( banner ) {
    banner.classList.add( 'dragover' );
    return false;
}

Webstorm's code inspection suggest me that Method can be static for the onDragOver method. My question is:

Are there any real benefit from having the method as static or this suggestion is somehow useless?

Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
Avraam Mavridis
  • 8,698
  • 19
  • 79
  • 133
  • The suggestion is issued because the method doesn't use any instance data. Of course, if you do have similar methods in other classes that do use instance data, and you might do here as well, you shouldn't make it static. – Bergi Jan 27 '16 at 15:15
  • @Bergi thx. That makes sense. And makes it more clear. – Avraam Mavridis Jan 27 '16 at 15:17

2 Answers2

13

Yes, you don't need an instance of the object when invoking a static function. All you need is a reference to the constructor:

class Foo {
  static bar() { console.log("foo"); }
}

Foo.bar(); // outputs "foo" to console

No need for a new Foo() anywhere.

By convention, instance methods should be used when you actually need state (either to read state, or to write state) from an instance.

The inspection will tell you that when you have a prototype/class method that does not have a this in it (and thus doesn't need an instance).

Madara's Ghost
  • 172,118
  • 50
  • 264
  • 308
  • So it is just a microptimization, right? To no create unnecessary instances. – Avraam Mavridis Jan 27 '16 at 15:08
  • 8
    No, it's a matter of what makes sense. I doesn't make sense to create a `new Math()` for `Math.random()`, there's nothing that would need an object's state. – Madara's Ghost Jan 27 '16 at 15:09
  • 6
    What if I call my Math.random() from inside the Math class ? Isn't it weird to call Math.random() instead of this.random() from another method inside Math ? – Growiel Apr 04 '16 at 03:12
  • 4
    It really isn't weird to call static methods using ClassName.method(), even from inside the class. Actually that's the way it's in pretty much all programming languages having static methods for classes. You should turn all those methods that do not use the state into static methods, not for the technical side of the things, but to communicate better the intention and possible side effects of said method/function. – Risto Välimäki Sep 29 '17 at 10:40
0

https://www.newmediacampaigns.com/blog/refactoring-react-components-to-es6-classes

http://aristid.es/react-es6-components-static-declarations/

Source to book

import {Image} from '.../some';
Image.fetch() //call static method

I think when different components can same use method then it's case for use static

zloctb
  • 10,592
  • 8
  • 70
  • 89