Skip to content

1st javascript task#4

Open
SaadOcloud wants to merge 8 commits intomainfrom
javascripttask
Open

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

Conversation

@SaadOcloud
Copy link
Owner

No description provided.

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)})

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

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

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?

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

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
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

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

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
Owner Author

Choose a reason for hiding this comment

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

Noted

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

let key='city';

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
Owner Author

Choose a reason for hiding this comment

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

Noted sir

Task1.js Outdated
{
var tempData = {};

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

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
Owner Author

Choose a reason for hiding this comment

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

Sure sir

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

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
Owner Author

Choose a reason for hiding this comment

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

Sure sir

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

}
tempData[array[index][key]].push(array[index])

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
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

Task2.js Outdated
const key='city';

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

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?

Task2.js Outdated
const key='city';

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

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.

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

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

Task2.js Outdated
acc[cv[key]]=[]
}
acc[cv[key]].push(cv)
tempobj=acc

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