Skip to content

1st javascript task#4

Open
SaadOcloud wants to merge 8 commits into
mainfrom
javascripttask
Open

1st javascript task#4
SaadOcloud wants to merge 8 commits into
mainfrom
javascripttask

Conversation

@SaadOcloud
Copy link
Copy Markdown
Owner

No description provided.

Comment thread Task1.js Outdated
function groupByKey(array, key) {
return array.reduce((hash, obj) => {
if(obj[key] === undefined) return hash;
return Object.assign(hash, { [obj[key]]:( hash[obj[key]] || [] ).concat(obj)})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you simplify it a lil bit

Comment thread Task1.js Outdated
@@ -0,0 +1,18 @@
function groupByKey(array, key) {
return array.reduce((hash, obj) => {
if(obj[key] === undefined) return hash;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

has key function or anything like this?

Comment thread Task1.js Outdated
@@ -0,0 +1,27 @@
function GroupByKey(array,key)
{
var tempData = {};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please don't use var (only use if its extremely necessary to do so.)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ok sir noted

Comment thread Task1.js Outdated
{id:4,name:"Rehan", city:"Lahore"},
{id:5,name:"Saqib", city:"Karachi"},
{id:6,name:"Farhan", city:"Islamabad"}
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use prettier with your VS Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Noted

Comment thread Task1.js Outdated
{id:6,name:"Farhan", city:"Islamabad"}
];

let key='city';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should be const instead off let

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Noted sir

Comment thread Task1.js Outdated
{
var tempData = {};

for ( var index=0; index<array.length; index++ )
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The logic works fine and is good.
But I want you to write another function that uses array.reduce method to do the same thing.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Sure sir

Comment thread Task1.js Outdated
@@ -0,0 +1,28 @@
function GroupByKey(array,key)
{
tempData = {};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

never declare a variable without using let or const.
Use const here as the object reference is never changed later on.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Sure sir

Comment thread Task1.js Outdated
tempData[array[index][key]]=[];

}
tempData[array[index][key]].push(array[index])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

array[index][key] this is used twice in the for loop so the sensible thing would be to assign it to a variable and then use it at both places. So later on if we need to make a change, we only need to make in 1 place.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ok Sir Let me resolve it

Comment thread Task2.js Outdated
const key='city';

function GroupByKey(arr,key) {
const tempData=arr.reduce((acc,cv)=>{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we really have to create this tempData variable?

Comment thread Task2.js Outdated
const key='city';

function GroupByKey(arr,key) {
const tempData=arr.reduce((acc,cv)=>{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When you take parameters make sure they are descriptive. I can't understand much by looking at this 'cv' variable.

Comment thread Task2.js Outdated
function GroupByKey(arr,key) {
const tempData=arr.reduce((acc,cv)=>{
if(!acc[cv[key]]){
acc[cv[key]]=[]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rehan bhai mentioned this before acc[cv[key]] can be stored as a variable because you are using it in more than on place

Comment thread Task2.js Outdated
acc[cv[key]]=[]
}
acc[cv[key]].push(cv)
tempobj=acc
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 cannot see the declaration of this tempObj where is this declared

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

Successfully merging this pull request may close these issues.

3 participants