DEV Community πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

Cover image for React.js Interview - technical submission and detailed feedback
Rajesh Royal
Rajesh Royal

Posted on • Updated on

React.js Interview - technical submission and detailed feedback

Update: The public links are down due to the company didn't wanted to have the assignment code shared. Sorry to say but the repo cannot be public anymore. Thanks for the everyone's feedback.

I have recently created a React.js project for one of the MNC I'm interviewing at and they provided a really detailed feedback of my assignment submission.

Sharing the codebase and the Feedback given by company here so anyone can benefit from it.

Requirement: Create a React.js app to list stocks and clicking on a stock will take to stock detail page which have stock quotes listing.

  • Should be filtrable.
  • Stock Quotes can be sorted by date.
  • Once the Quotes expires fetch new quotes [polling].
  • Should use React.js.

Submission: (its private now)

πŸ“ƒ Detailed Feedback

@Negatives
- code look messy and unnecessarily made complicated
- created a common http get service around axois, the implementation is not proper
- unnecessary component wrappers, just for using some CSS classes
- usage of error boundary is not in the recommended way
- unnecessarily added new functionality rather than concentrating on the complete solution
- unnecessarily suppressing lint rules
- not handling mobile responsiveness

@Instruments page / stock listing
 - api call twice on page mount
 - using 2 different state for search results and default view
 - filter function will always runs

@Quotes page / stock detail page
 - api call twice on page mount
 - polling is implemented but the implementation is messy and won't work
 implementation details
      - spawn a web worker and listen for post message, inside this if the quotes list length is 0 make an api call to update the quotes
      - providing the user a control to update the interval (1,2,3,5,10 ms) for checking expired quotes
      - there's  a  set interval run on this interval (1..10 ms) inside this posting a web worker message
      - web worker will run the loop for checking the quote's expiry and the expired leg is removed from the list
      - once all items have been removed from the list, initiate the api call to fetch new quotes, this time frontend will hit the server at least 100-300 times (sometimes more than 1800 requests)       reasons for this the web worker will receive a message every 1-10ms and hit the server
 - not properly clearing set interval, leads to calling the previous apis as well, now the loop will hit the server more than 5k times
 - Use string split and replace T and Z to convert the timezone for comparing the time
 - timestamp not converted to IST in the table listing

@Positives
- Using typescript
- Using error boundaries
- segregating code by spliting components and util functions
Instruments page
- implemented search
Quotes page
- implemented sort

We thank you for the time, energy, and effort you invested in our process and we wish you the very best in your future endeavours.
Enter fullscreen mode Exit fullscreen mode

Β 

Disagree Points.

  • What I didn't found helpful in this feedback is that it says the code looks messy, which I totally don't agree. Folks here can give their feedback about this and this will be super helpful.
  • Also The react does render everything 2 times in dev mode in v18, that's why APIs get called two times, but in production it doesn't happen.
  • The timestamp which is received by API must be in Unix timestamp or UTC to do date operations better, I had tough time doing that time conversion which I end up doing with split and replace. Didn't wanted to use moment for this 1 task.
  • filter function will always run: what I'm suppose to do here, the filter function should run every time user type anything in the s search field.
  • usage of error boundary is not in the recommended way - what is the recommended way ?

At the end It was a really nice experience and get to know new things.

Thank you for reading πŸ€“

Top comments (33)

Collapse
manuartero profile image
Manuel Artero Anguita

Yet another case of "demanding rampage" (I made up this term). Establishing the measuring stick far away from their daily work. This is such a good interview project :)

good work!

((I'd say it's actually better for you not entering a place where they have this interview process)).

Collapse
hnrq profile image
Henrique Ramos

Would you mind explaining why is it better not to enter such a place?

Collapse
manuartero profile image
Manuel Artero Anguita

πŸ€” I'll try!

The selection process is the very first contact the candidate has with the company culture. I mean the actual culture (cause every company has the most incredible values on the paper).

  • A highly demanding selection process - solve this entire project at home, for free - will be the first clue about their culture.
  • A highly demanding selection process + demanding perfection....
  • A highly demanding selection process + demanding perfection + subjective evaluation...
Thread Thread
ninjainpajama profile image
ninja in pajama • Edited on

Got your point man, but what other options companies do have to access a candidate. I really like when companies ask me about my projects, challenges all other things and grind in details with tech rounds of logic building.

I also do not like doing take home assignments, It should only be in practice if anyone is hiring for fresher positions.

Collapse
shinspiegel profile image
Jeferson 'Shin' Leite Borges • Edited on

Your code is a little bit messy.
Use this to learn, improve the quality.

Let me add some details on this,

  • On this folder you have files using camel case and kebab case. I don't care which one is used, but be consistent.
  • This file is just an exporter. In this case, this file should follow the same rule.
  • This is a little bit over complex without returning value to the project.

There is extra stuff,
For a regular developer this is not expected, but for a senior position, this type of care is expected, in special if you are going to mentor other developers.

Collapse
rajeshroyal profile image
Rajesh Royal Author

⚫ Second Point.
The default exporter of containers is created because there can be multiple exports of containers. [ex. - HOC for error boundary, HOC for Auth Provider and etc.] That's why it have only exports. But in case of a particular page there will be only page itself to be exported.

⚫ Third Point.
I accept it looks a little bit complex for just 2 routes, I could have just added two simple routes. But keeping the scalling of routes in mind I implemented it this way so It will be more easy to add childrens to the routes anytime in future.

Your feedback is really helpfull, what idea I get from the company feedback is that I should be doing what needed and in simply ways, instead of thinking the future prespectives. [deliver what needed πŸ˜„]

Thanks @shinspiegel

Collapse
shinspiegel profile image
Jeferson 'Shin' Leite Borges

I was afraid that you get very pissed by my comment.
Thanks for understanding. :D

Thread Thread
ninjainpajama profile image
ninja in pajama

πŸ˜…

Thread Thread
rajeshroyal profile image
Rajesh Royal Author • Edited on

@shinspiegel hehe.. It looks like that when you start reading from top 🀣🀣

Collapse
alvinpeter9 profile image
alvinpeter9

Thanks, I have learnt from your explanation as well

Collapse
pyrsmk profile image
AurΓ©lien Delogu • Edited on

I recently passed this type of test for a company, in Ruby. It said it would take up to 4 hours but I ended taking 7 hours to complete it (the statement wasn't clear enough, and I made the test in TDD, which add more time than expected because I needed to consider what missing rule from the statement I should add or not).

Finally, they did not even took the time to make a feedback and I just had a vague explanation from the HR.

I was a bit vexed and angry, but anyway, I took that code to pass another type of test for another company: they want me to take some code and explain it live. It was really fresh in my mind and I could explain clearly all my choices. The CTO was really comprehensive and clear about the mistakes I made. Never say the least, this company hired me as a senior developer and tech lead. So, indeed, everything can be a valuable lesson!

Collapse
rajeshroyal profile image
Rajesh Royal Author

agreed.

Collapse
airtonix profile image
Zenobius Jiricek • Edited on

You left console.log in there. I'd block your pr if you tried to push that though (infact our company has dangerjs checks that do this for us)

They called it messy because:

  • There's unnecessary use of promises.
  • you used a lot of inline anon functions

If you're going to separate the components, start putting each component in it's own folder with the styles (use CSS modules).

You used a folder called containers, but it didn't contain any data layer or network layer just the error boundary.

That's all I noticed, I'm tired and about to head off to sleep.

If this was for a junior position, I'd hire you in a flash... You have promise - technically.

But then next step is - How you take criticism... It's more important than your perceived technical prowess.

If it was for a senior position then you've a long way to go before you start getting upset that you were rejected.

Collapse
rajeshroyal profile image
Rajesh Royal Author

Thanks @airtonix It helpfull, I will try If i can optimize it.

And for consoles I personally use git pre-commit hook to block committing a code, but I didn't felt need to implement it here. But I should have removed them.

Collapse
rajeshroyal profile image
Rajesh Royal Author

@airtonix if you can point out where is unnecessary use of promises exactly that will be really helpful.

Collapse
krishnakantsalkar profile image
Krishnakant Salkar

For filter function runs everytimr or search methods you could use rxjs typeahead flow a very good example given here: learnrxjs.io/learn-rxjs/recipes/ty...

Rxjs can be a valuable addition to any js project

Collapse
rajeshroyal profile image
Rajesh Royal Author

Thanks @krishnakantsalkar I will definately check it out.

Collapse
pengeszikra profile image
Peter Vivo

Add debounce functionality for filter, figure out useDebuounce custom hook, this will be helpful in long term.

Collapse
rajeshroyal profile image
Rajesh Royal Author

I have added a todo for that, I was about to di it but later decided to not do it because It wasn't making any much difference. But yes still good to implement.

Collapse
ninjainpajama profile image
ninja in pajama

I'm glad that you at least get feedback πŸ˜„

Collapse
rajeshroyal profile image
Rajesh Royal Author

lol πŸ˜…πŸ˜…

Collapse
makranelhoucine profile image
EL HOUCINE MAKRAN

For your information

ReactJs render twice in development mode because of strict mode react component

Remove StrictMode component in index.ts then ReactJs will render only one time.

Collapse
yongchanghe profile image
Yongchang He

Thank you for sharing this!

Collapse
rajeshroyal profile image
Rajesh Royal Author

Thanks @yongchanghe

Collapse
mattdevio profile image
Matt G

This is a part of the me that says coding challenges need to go and are a red flag. Bullet dodged?

Collapse
konoplitskii profile image
Sergey

Thank a lot for article.

Collapse
topninja profile image
Michael Jin • Edited on

I can agree to use ts with their feedbacks at least.

Collapse
rajeshroyal profile image
Rajesh Royal Author

hehe πŸ˜„πŸ˜„

Collapse
rajeshroyal profile image
Rajesh Royal Author

Thank you for finding it helpful I appreciate that but you can do your SEO somewhere else instead of destroying the beauty of dev platform. Hope you understand.

Collapse
ogranada profile image
Andres Granada

Hello, first of all, great work, many things about great coding here.
IMHO I think you have a great knowledge about typescript and the generics, that is great, but trying to make scalable you added many things that could be easier for a 2 endpoints project.
You could add some extra details, being careful creating the http instance using a singleton pattern (isolation by module is good but isolation by instance is better). API double calls could be related with the useEffect return statement (useStockListingTable.tsx have no clean function, check dev.to/ag-grid/react-18-avoiding-u...).

In general I enjoyed check your code, many react magic things, just some details. Also, no all the companies have the same criteria to work, therefore, learn from this and point to new companies that need your talent. Thanks for sharing.

Collapse
rajeshroyal profile image
Rajesh Royal Author

Thank you for reading @ogranada
The API dual call happen in only dev mode of React V18. In prod build everything is working perfect.

If you can explain a little bit about it or share some resources I would love to learn - (isolation by module is good but isolation by instance is better)

Collapse
makranelhoucine profile image
EL HOUCINE MAKRAN

Because of StrictMode component used in index.js/ts

If you remove it ReactJs will do only one render not twice in development also

🌚 Browsing with dark mode makes you a better developer.

It's a scientific fact.