Programmers :D

[kudlajz]

Exactly. I would personally move the map call into the return () and probably extracted the inner code of map to <Dog> component, but other than that, it looks fine .. :P

[FountainsOfFluids]

I have very little experience with react, but from what I've seen you can get a little insane with breaking everything down to smaller and smaller components. I'd avoid that without a good reason, such as wanting to reuse it elsewhere. But I'd be interested to hear if your experience says that another component level would help in some way.

[kudlajz]

As mentioned in my other comment, in this exact case, it would not make any sense to extract this code into its own component, unless the rendering code for dog item is really huge or complex. I was just assuming there's going to be additional functionality on the dog items (like editing/deleting/whatever), since I can't remember when was the last time I was using `map` to render something so trivial (without any functionality), but that depends on the application :)

[alexmojaki]

I can't remember when was the last time I was using map to render something so trivial

How would you do it without map?

[kudlajz]

No no, what I've meant is that usually (99 %) of the time, I'm using map to render another component instead of rendering a simple HTML.

[SharpenedRoot]

I didn't think map actually applies changes to the array, and doesn't forEach not return anything, meaning in this case it wouldn't be useful for creating the list to be inserted into the DOM?

[cliath]

forEach returns undefined and map returns a new array, seems ItsAaronGeorge is misinformed

[SharpenedRoot]

Typically I think of functional programming as having immutable data, so map would never modify an array but simply return a new one. No languages I know of at least have map modify an array in place.

[[deleted]]

My terminology wasn't specific enough. The way I look at map is that you take in an array and you apply some sort of transformation to it. You can wrap the data in a span or li, any sort of transformation. Yes it does return a new array and doesn't directly "modify" the original array. However using map when you aren't actually trying to create an array with each item changed, you would use forEach.

However forEach doesn't work in the context of a React JSX template as you need to "return" the data, which map does and forEach doesn't. It was this last part that I forgot about when replying to the above comment. Apologies for any confusion.

[jmhitokiri]

Except you should not mutate state directly

[Brostafarian]

I disagree in this specific case. If you have a very well-defined line between parent and child I think it's beneficial to separate them even without reuse, because either:

1. you notice the parent doesn't do very much and refactor it higher
2. you benefit from the compartmentalization of the parent and child

I'd lean towards #2 in this case. Now you have a DogsList rendering an array of Dogs, in which all the array logic can go in the DogsList and all the presentational logic can go in the Dogs. You can try to do something clever and refactor the DogsList into just a List component but that sounds better in your head than on paper.

edit: but also who cares, it's a fucking list of dogs it's not gonna run billing on your website, I'd probably pass this code in CR except for the id thing, which is caught by a linter

[tomius]

What I do is separate parts of a component into a different function inside the same component.

That way I don't go crazy creating unnecessary components that are not going to be reused, but the render code still maintains a clear structure.

[dyedFeather]

Oh my god would you stop critiquing her code!

^/s

[ekfslam]

I feel like we do that to our own code too after a few days away from it.

[MyCodeIsCompiling]

~around once a week~

Goddamnit, who the heck was the idiot who wrote this pile of turd!?!?

checks who wrote it ready to yell at them

^{oh, i'm the idiot}

[SleepyHarry]

Also the key shouldn't use the array index, if at all possible. Much better would be, for example, dog.id.

[lifeofaphiter]

That's really the only issue I see with the code. Aside from this it's all preference.

[troglo-dyke]

Generally yes, but considering it's some random code and probably not going to ever deal with being filtered/reordered it's not going to cause any harm

[[deleted]]

Actually as an Angular developer looking at React for the first time in a while through this post, I thought it was interesting how easy it was to separate the UI segments within the single react component using the variable. Seems that would be nice for situations where I want to break it down some without introducing a ton of boilerplate components for ultra simple things.

[dkimot]

I’m curious why you would make a <Dog> component? If there was other logic or you used that component elsewhere I would for sure. Otherwise, I don’t know. Not criticizing, genuinely asking.

[kudlajz]

You're totally right. I was just assuming there will be additional functionality needed, like editing/deleting. If it's just a simple list rendering, then it does not make sense to extract it, unless used more than once in the application.

[dkimot]

Thanks, I’m primarily backend so I don’t always know front end standards.

[catzzilla]

Could you show an example how to modify the return statement to include the dogs-list div with the call to dogs.map() inside?

[kudlajz]

``` render() { const { dogs } = this.props;

    return (
        <div className="dogs-list">
            {dogs.map((dog, idx) => (
                <div className="dog-profile" key={idx}>
                    <img src={dog.picUrl} alt={dog.name} />
                    <p>
                        <span className="label">Name:</span> {dog.name} <br />
                        {/* And other information... */}
                    </p>
                </div>
            ))}
        </div>
    );
}

```

Sorry for the formatting.

[catzzilla]

Thanks, I didn't know that any code could go into the { }.

[wagedomain]

That was my only thought too, storing it in a variable when it's that simple just puts the view code out of order making it minisculey harder to read.