Skip to content

assigment by shivamg11000(shivam gupta)#5

Open
shivamg11000 wants to merge 1 commit intojavascripters-community:masterfrom
shivamg11000:assignment_by_shivam_gupta
Open

assigment by shivamg11000(shivam gupta)#5
shivamg11000 wants to merge 1 commit intojavascripters-community:masterfrom
shivamg11000:assignment_by_shivam_gupta

Conversation

@shivamg11000
Copy link
Copy Markdown

done the assignment, took the same idea of e-commerce and implemented it.

@upa8
Copy link
Copy Markdown
Contributor

upa8 commented Jul 27, 2017

Good Job @shivamg11000.

We will review it after the contest.

Copy link
Copy Markdown

@akshaynaik404 akshaynaik404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert components using Class syntax to Stateless functional Components and use camelcase naming convention.

import React from 'react';
import './Cart.css';

class Cart extends React.Component{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Component can be converted to a Stateless Functional Component. Just like the others. For more info Click Here

}

handleAddToCart(itemKey){
const no_of_cart_items = this.state.cart.no;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Camel case is preferred in JavaScript based projects. Corresponding Airbnb style guide rule

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know but for such big variables snake case is more readable

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this react lifecycle method then - `componentWillReceiveProps()'?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ofcourse we have to use it , snakecase will not work here

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to convey that "componentWillRecieveProps" is long variable name and yet camel case is used here.




class OrderSummary extends React.Component{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class can also be converted to Stateless functional component.


import './Shop.css';

class Shop extends React.Component{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class can also be converted to a Stateless functional components


import './ShopItem.css';

class ShopItem extends React.Component{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert all classes that either does not have state or won't have state in future, to a stateless functional component.

Copy link
Copy Markdown
Contributor

@javascripters2015 javascripters2015 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment wherever required to understand the code implementation intention
Make code more readable
Update README file with all required instruction to setup project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants