Trying to optimise my code in React, but stuck :/

Hello!

Today I was trying to optimize my code, but i’m actually stuck because i cant use function from another file.

What i’ve done so far, is that I created a folder ''component", and inside, created a file name " Api.js "

Here’s the code:

import React, {Component} from 'react';
import axios from 'axios';

class Api extends Component {

    constructor(props) {
        super(props);
        this.state = {
            items: [],
            summonername: '',
            token: "Tokenmagic",
            id: '',
            champions: [],
            champion:[],
            name: [],
            golds: [],
            SortGold:[],
            MvpGolds: [],
        };
        this.GetMaxValue = this.GetMaxValue.bind(this);
        this.apiSummoner = this.apiSummoner.bind(this);
        this.apiAccount = this.apiAccount.bind(this);
        this.apiGame = this.apiGame.bind(this);
        this.onClick = this.onClick.bind(this);
    }
        async apiSummoner() {   // Get summonner information
        let summonername = this.state.summonername;
        const summonerurl = `/summoner/v4/summoners/by-name/${summonername}`;

        await axios.get(summonerurl, { //Attendre le résultat de la promesse d'axios
            params: {}, headers: {
                'X-Riot-Token': this.state.token
            },
        })
            .then(res => {
                console.log(res);
                this.setState({summonersname: res.data.name});
                this.setState({summonerslevel: res.data.summonerLevel});
                this.setState({summonersid: res.data.accountId});
            })
            .catch(error => {
                console.log("An error occured on  apiSummoner D: ", error);
            });

    };


    async apiAccount() { // get Last matchId of the summoner
        let summonerId = this.state.summonersid;
        const matchurl = `/match/v4/matchlists/by-account/${summonerId}`;
        await axios.get(matchurl, {
            params: {},
            headers: {'X-Riot-Token': this.state.token}
        })
            .then(res => {
                console.log(res);
                this.setState({matchId: res.data.matches[0].gameId});
                this.setState({matchchampion: res.data.matches[0].champion});
                this.setState({matchlane: res.data.matches[0].lane})
            })
            .catch(error => {
                console.log("An error occured on apiAccount D: ", error)
            })
    }

    async apiGame() {
        let matchID = this.state.matchId;
        const gameurl = `/match/v4/matches/${matchID}`;
        await axios.get(gameurl, {
            params: {},
            headers: {'X-Riot-Token': this.state.token}
        })
            .then(res => {
                for (let i = 0; i < 10; i++) {
                    this.setState({
                        name:
                            [...this.state.name, res.data.participantIdentities[i].player.summonerName]
                    });
                    this.setState({
                        golds:
                            [...this.state.golds, res.data.participants[i].stats.goldEarned]
                    });
                    this.setState({
                        champion:
                            [...this.state.champion,
                                getChampionById(this.state.champions, res.data.participants[i].championId)]
                    });
                }
                this.setState({bluetowerdestroy: res.data.teams[0].towerKills});
                this.setState({redtowerdestroy: res.data.teams[1].towerKills});
                this.setState({bluedragonkilled: res.data.teams[0].dragonKills});
                this.setState({reddragonkilled: res.data.teams[1].dragonKills});
            })
            .catch(error => {
                console.log("An error occured on apiGame D: ", error)
            })

    }

    GetMaxValue() {
        let Mvp = Math.max.apply(Math, this.state.golds);
        for (let i = 0; i < 10; i++) {
            if (Mvp === this.state.golds[i])
                this.setState({MvpGolds: [...this.state.MvpGolds, this.state.golds[i] + "-> MVP"]});
            else
                this.setState({MvpGolds: [...this.state.MvpGolds, this.state.golds[i]]});

        }
    }

 componentDidMount() {
        fetch('http://ddragon.leagueoflegends.com/cdn/10.9.1/data/en_US/champion.json')
            .then(response => response.json())
            .then(res => {
                this.setState({champions: res.data});
            })

    }

    async onClick() {
        try {
            await Api.this.apiSummoner();
        } catch (e) {
            console.log("error : ", e)
        }
        try {
            await this.apiAccount()
        } catch (e) {
            console.log("error", e)
        }
        try {
            await this.apiGame()
        } catch (e) {
            console.log("error", e)
        }
        this.GetMaxValue()
    }

}
function getChampionById (data, id) {
    let dataKeys = Object.keys(data);
    for (let key of dataKeys) {
        if (data[key].key == id ) {
            return data[key].name;
        }
    }
    return {error: "Champion not found"}
}


export default Api;

And, thanks to those value get from the api, I use them is my app.js and render them.:

import React, {Component} from 'react';
import axios from 'axios';
import { Bar } from 'react-chartjs-2';
import Api from './components/Api.js';

class App extends Component {

    
    render() {
        const data = {
            labels: [this.state.name[0], this.state.name[1], this.state.name[2], this.state.name[3], this.state.name[4],
            this.state.name[5], this.state.name[6], this.state.name[7], this.state.name[8], this.state.name[9]],
            datasets: [
                {
                    label: 'Gold',
                    backgroundColor: 'rgb(255,215,0)',
                    borderColor: 'rgb(255,215,0)',
                    data: [this.state.golds[0], this.state.golds[1], this.state.golds[2],
                        this.state.golds[3], this.state.golds[4], this.state.golds[5], this.state.golds[6],
                        this.state.golds[7], this.state.golds[8], this.state.golds[9]],
                }
            ]
        };
        return (

            <div>
                <label className={"label"}>
                    <input type="text" onChange={this.updateInput}/>
                    <input type="submit" onClick={Api.onClick} value="Search"/>
                </label>
                <h1>Summoner Information</h1>
                <div> Name : {this.state.summonersname}  </div>
                <br/>
                <div> Level : {this.state.summonerslevel} </div>
                <br/>
                <h1> Last Match : </h1>

                <br/>
                <h2> Blue Side </h2>
                <h4> Summoners: </h4>

                <div> : {this.state.name[0]} -> {this.state.champion[0]} -> {this.state.MvpGolds[0]} Golds </div>
                <div> : {this.state.name[1]} -> {this.state.champion[1]} -> {this.state.MvpGolds[1]} Golds </div>
                <div> : {this.state.name[2]} -> {this.state.champion[2]} -> {this.state.MvpGolds[2]} Golds </div>
                <div> : {this.state.name[3]} -> {this.state.champion[3]} -> {this.state.MvpGolds[3]} Golds </div>
                <div> : {this.state.name[4]} -> {this.state.champion[4]} -> {this.state.MvpGolds[4]} Golds </div>
                <h4> Stats : </h4>
                <div> : Dragon killed: {this.state.bluedragonkilled} </div>
                <div> : Tower Destroid: {this.state.bluetowerdestroy} </div>
                <br/>
                <h2> Red Side </h2>
                <h4> Summoners: </h4>
                <div> : {this.state.name[5]} -> {this.state.champion[5]} -> {this.state.MvpGolds[5]} Golds</div>
                <div> : {this.state.name[6]} -> {this.state.champion[6]} -> {this.state.MvpGolds[6]} Golds</div>
                <div> : {this.state.name[7]} -> {this.state.champion[7]} -> {this.state.MvpGolds[7]} Golds</div>
                <div> : {this.state.name[8]} -> {this.state.champion[8]} -> {this.state.MvpGolds[8]} Golds</div>
                <div> : {this.state.name[9]} -> {this.state.champion[9]} -> {this.state.MvpGolds[9]} Golds </div>
                <h4> Stats : </h4>
                <div> : Dragon killed: {this.state.reddragonkilled} </div>
                <div> : Tower Destroid: {this.state.redtowerdestroy} </div>
                <div>
                    <h2>Game Gold</h2>
                    <Bar ref="chart" data={data} />
                </div>
                <br/>


            </div>
        );

    }


}
export default App;

So I tried to follow mutiples things like : https://create-react-app.dev/docs/importing-a-component/ or https://www.youtube.com/watch?v=iWcNI4PIjVU, but react return me for example Cannot read property 'name' of null, from this.state.name[0] because he dont know what data it is.

Any help would be very appreciated, thanks !

The API isn’t a component here, you should just be writing a JS class (or just a bunch of individual functions). React has a small API that deals specifically with taking data and building nodes to be rendered to HTML – it doesn’t care about how you fetch data or anything like that.

At the minute, what you’ve tried to do is jam a load of async fetch stuff into a component, but it isn’t actually a react component, which is a function that takes some props as input and returns some JSX (which in turn converts to a set of functions that are used to create nodes, which are used as input to the ReactDOM library functions to render HTML).

Can you explain what you’re doing, and what you want to happen – what’s the input here that you want, what’s the output. I can walk you through refactoring at least some of it, but it would be helpful to know that before I advise anything

Edit after reading the code a bit:

So the reason for splitting out the API, ideally, is to decouple all that quite complex logic from the component that renders the data. The two shouldn’t know anything about each other. This lets you focus on just making sure that your data fetching works without needing to couple it to React and its complex rendering logic. So it is a very good idea to do what you’re doing.

It makes sense to use a class as well, because you initialise it by making a fetch call ( http://ddragon.leagueoflegends.com/cdn/10.9.1/data/en_US/champion.json). That populates the internal state with some values. Once it’s initialised, you can then use those values to make further queries. The API then exposes a method to do that (and could expose more – a benefit of this is that you can easily expand it)

Hey Dan!

Thanks for taking your time to help me.

So what I’m doing here is pretty simple.

First of all, I get all the data of the champion( League of legends) from the fetch call that you saw., then I’m doing 3 api call, the first one API summoner witch get the data of the user you want to search:

  • Name
  • Level
  • AccountId which I’ll use in apiAccount to get the information of the last matchs he done.
    And thanks to apiAccount, I’ll get a ‘‘gameId’’ which I’ll use in apiGame and it will give me more information about this game, the duration, the total gold of a participant in the game, ect …

And here I’m trying to render the information about the player, and the first match he done, with a couple informations about it ( Other participants, name , …)

So I should just take everything from my render and put in in my Api.js?(because the two files shouldn’t know anything about each other )

So with that data, do you only need to get it once for the whole app? As in, once you’ve made that call, the app is just going to use that data.

The API module can just be a class, nothing to do with React: you initialise it with that first call, then you can use it to make your queries.

But afaics you’re only making a single query. The point of moving the API out would be to put a wrapper over the calls and provide functions to get the data you want in a form you want. So like

class Api {
  items =  [];
  summonername = "";
  // etc...

  constructor() {
   // make fetch call for initial data, populate `this.champions`
  }

 async  doTheThingYouWantToDoOnClick () {
    // some fetch calls, which populate the relevant properties defined at the top
    // return the data in a form that is useful to you
  }
}

// Then possibly this, though there are a few ways to
// approach it -- basically you want to make sure the initial
// data, the important bit, is fetched as the first thing your app does
const api = new Api();
export default api;

Now you import that, and you can call api. doTheThingYouWantToDoOnClick() to get data in the form you need it

Yes exactly !
I fetch those datas, sort them and use them,

But how can i just get them and tell to the other file: Here is the datas I fetched, display them
That’s what I’m trying to do but he doesnt know them 0o?
Should I create a render in my api file and use it in app.js?

Sorry, edited that answer slightly.

Where I would start is in the App component. Get a sample of the data – it’s going to be an object, so at the top of the file just have like const data = { // just paste a full response that you'd expect from that search in }. Make the component work with this mock data first – you’re going to replace it with a function call to the actual API in a bit, but as that call resolves to the same shape data, they are completely interchangeable. So instead of Api.onClick, have something like {async () => await theDataObject } (or write another function at the top that does the same thing, and call that). Note that it’s async – this is important because the API call is async, and you want it to behave the same way (it also lets you put a delay in pretty easily to mimic a slow connection)

Second thing once you’ve got that working is split out the rendering of your response to a seperate component – this can be a function that takes that response object as a prop and renders it. It’s exactly what you have, just pulled out. Then in your render, replace what you have to render the response with <ThatNewComponent data={data} />.

So the render() should now have the search input, then that component. This isn’t totally necessary, but it’s generally a good idea – it means you have a small simple component to work with instead fiddling with stuff inside a large, complex component, and it’ll make it easier to ensure it only renders data when the data actually arrives

EDIT: what all this is doing is seperating the logic for the rendering (React) from the logic you’re going to have to figure out for fetching and parsing the data from the games API.

If you work on the React part first, you can just build it using this mock data, then replace the mock calls with real calls and it should (:crossed_fingers: ) work fine. It means you only need to think about one thing at a time

You aren’t going to put any render logic in the API module, it doesn’t need it.

Hello!
Thanks for this big answer 0o

So I understood the part with the render and the async function.
But… the logic that you are trying to teach me I dont really understand it.

So, what I need to do is to pass the reponse of my differents api call in my const data ( So in my api.js file) and then I’ll can use them in my app.js and render them?
(I’m sorry, this is probably easy to do but I’m very new in programming so I’m a bit lost)

So, at the minute, you’re trying to make a call to an API, you’re using Axios to do that. You make the call, load in data, handle that.

What I’m saying is, start by just passing the data: literally forget about the logic around the API call until it all works by just passing an object to your components.

At the minute you’re trying to do a hundred things at once, and you need to keep all of those things in your head.

I’m saying get rid of all the other things, just do one thing at once.

import React, {Component} from 'react';
import axios from 'axios';
import { Bar } from 'react-chartjs-2';
import Api from './components/Api.js';

const SOME_DATA = {
  // Whatever should come back when a
  // User clicks search
};

class App extends Component {
    render() {
        const data = {
            labels: [this.state.name[0], this.state.name[1], this.state.name[2], this.state.name[3], this.state.name[4],
            this.state.name[5], this.state.name[6], this.state.name[7], this.state.name[8], this.state.name[9]],
            datasets: [
                {
                    label: 'Gold',
                    backgroundColor: 'rgb(255,215,0)',
                    borderColor: 'rgb(255,215,0)',
                    data: [this.state.golds[0], this.state.golds[1], this.state.golds[2],
                        this.state.golds[3], this.state.golds[4], this.state.golds[5], this.state.golds[6],
                        this.state.golds[7], this.state.golds[8], this.state.golds[9]],
                }
            ]
        };
        return (

            <div>
                <label className={"label"}>
                    <input type="text" onChange={this.updateInput}/>

HERE:

                    <input type="submit" onClick={async () => await SOME_DATA} value="Search"/>
                </label>
                <h1>Summoner Information</h1>
                <div> Name : {this.state.summonersname}  </div>
                <br/>
                <div> Level : {this.state.summonerslevel} </div>
                <br/>
                <h1> Last Match : </h1>

                <br/>
                <h2> Blue Side </h2>
                <h4> Summoners: </h4>

                <div> : {this.state.name[0]} -> {this.state.champion[0]} -> {this.state.MvpGolds[0]} Golds </div>
                <div> : {this.state.name[1]} -> {this.state.champion[1]} -> {this.state.MvpGolds[1]} Golds </div>
                <div> : {this.state.name[2]} -> {this.state.champion[2]} -> {this.state.MvpGolds[2]} Golds </div>
                <div> : {this.state.name[3]} -> {this.state.champion[3]} -> {this.state.MvpGolds[3]} Golds </div>
                <div> : {this.state.name[4]} -> {this.state.champion[4]} -> {this.state.MvpGolds[4]} Golds </div>
                <h4> Stats : </h4>
                <div> : Dragon killed: {this.state.bluedragonkilled} </div>
                <div> : Tower Destroid: {this.state.bluetowerdestroy} </div>
                <br/>
                <h2> Red Side </h2>
                <h4> Summoners: </h4>
                <div> : {this.state.name[5]} -> {this.state.champion[5]} -> {this.state.MvpGolds[5]} Golds</div>
                <div> : {this.state.name[6]} -> {this.state.champion[6]} -> {this.state.MvpGolds[6]} Golds</div>
                <div> : {this.state.name[7]} -> {this.state.champion[7]} -> {this.state.MvpGolds[7]} Golds</div>
                <div> : {this.state.name[8]} -> {this.state.champion[8]} -> {this.state.MvpGolds[8]} Golds</div>
                <div> : {this.state.name[9]} -> {this.state.champion[9]} -> {this.state.MvpGolds[9]} Golds </div>
                <h4> Stats : </h4>
                <div> : Dragon killed: {this.state.reddragonkilled} </div>
                <div> : Tower Destroid: {this.state.redtowerdestroy} </div>
                <div>
                    <h2>Game Gold</h2>
                    <Bar ref="chart" data={data} />
                </div>
                <br/>


            </div>
        );

    }


}
export default App;

What I’m saying to do is that when the thing is clicked, just return the data you’d expect to get, dont make an API call.

It will not work, and you need to make it work before you do anything else

Ok !

So if I’m not wrong, for example, I should just start by passing my data ‘name’ in my app component and not every of my data because i’m gonna get lost.

But I have one last question, what do you mean by :

What I’m saying is, start by just passing the data: literally forget about the logic around the API call until it all works by just passing an object to your components.

So I need to do the api call on api.js , pass only for example the data name[i] and render them?

Edit:

I read again your message
*dont* make an API call. So I need to pass a random data and test by my self if that work and understand this thing ?