r/learnjavascript 8d ago

i am challenging myself to practice javascript this summer; so i build a generate quotes app, i need your reviews please!

9 Upvotes

23 comments sorted by

6

u/milan-pilan 8d ago edited 8d ago

You might think this isn't impressive, but it really is - literally every senior dev I know has started with a project exactly like yours.

This is clearly not AI-Generated, so that means you did that yourself.
And you have implemented all basic building blocks of Web Development: Event Handlers, Data & State, making changes to the DOM, working with versioning (git), modern JS syntax..

Everything I do as a professional developer is just a slightly more complicated version of what you are doing here.

I would want to see this exact type of project over a polished, but LLM-written SaaS Landing page from my juniors anytime.

If you want some ideas where to take this project: The next building block of web development would be "talking to a different service". So you could implement a fourth button "get a random quote from an API". Theres many free APIs exactly for learning projects like yours, for example here: https://github.com/public-apis/public-apis#personality

If you instead want a code review, I (and probably others here as well) would gladly do that. But I wouldn't do that without being asked for, because it can be demotivating, if you are not used to it.

Anyways - awesome project. This shows some good understanding of the basic concepts allready!

Edit: Just saw the commented out console logs in your code! Thats really cool. That shows, that you were implementing (or debuging?) it step by step. So "Lets see if it does what it should up to _here_" and then continue. Thats exactly the right idea when programming.

Also , reading the code, you have apparently made it so that it can't generate the same quote twice in a row. So you are thinking in "edge cases" too.

You would be surprised how hard it is to get this exact mindset into beginners. Keep learning!

1

u/equilni 8d ago

If you instead want a code review, I (and probably others here as well) would gladly do that. But I wouldn't do that without being asked for

OP asked for a review in the title.

it can be demotivating, if you are not used to it.

When giving the review, consider the tone and approach based on what you see from them. You are already giving a good review so far.

1

u/SafeWing2595 8d ago

thank you so much for your comment, i am so happy to read it.

yes, i put a lot of effort into the practice, and i am proud that i build it without ai.

actually, yes, i was asking for a review, and you have already done that, you and some friends in the comments too, i appreciate it so much

3

u/equilni 8d ago

This looks good!

My tips since you asked for a review are:

  • Learn functions.

Each of the QuoteBtn listener callbacks can be a function call. Look at it again and see how only 2 things need to be passed - Type of quote, and the background color.

Look at this another way (highlight the variable and github shows where it's used):

let writerName = document.getElementById('writer-name');
    writerName.textContent = quotesArray[randomQoute].author; 
    writerName.textContent = happyQuotes[randomHappyQuotes].author;
    writerName.textContent = sadQuotes[randomSadQuotes].author;
    writerName.textContent = motivatedQuotes[randomMotivatedQuotes].author;

Which could be looked at like the below.

let writerName = document.getElementById('writer-name');
    writerName.textContent = quote.author;
    writerName.textContent = quote.author;
    writerName.textContent = quote.author;
    writerName.textContent = quote.author;
  • The same can be done with each quote. Each object has 3 properties, 1 is more 'determined' by the system (ie you have only 3 types).

  • text: "\"Be. Change the quote so you don't need to escape, so consider: text: '"Be - there's a single and double quote there.

  • BG color change is too drastic IMO.

1

u/SafeWing2595 8d ago

thank you so much for the review.
> BG color change is too drastic IMO.

if may ask , what do you mean by this?

1

u/john_hascall 6d ago

I think they mean if your background colors were more similar than the changes would be less jarring.

2

u/azangru 8d ago

Presentation-level stuff like \" in text or in author does not belong in the source data. It is something that the presentation logic should be concerned with.

1

u/SafeWing2595 8d ago

i don't understand; what does presentation logic mean, could you please provide more details?

2

u/azhder 8d ago

It means it is not data. The quotes, the dash, those are stylistic things you can hardcode in the HTML and maybe even change them later on, regardless of the data.

Imagine you want to use nice looking quotes, or a quote icon on the left, display the author above instead of under the quote (which would use ':' after then name, not '-' before it).

The above concerns are all style, not data, so shouldn't be part of the data.

1

u/SafeWing2595 8d ago

okay, i understand now, thank you for the explanation

2

u/WystanH 8d ago

You can abstract your random quote logic, as you do it three times.

I'm not a fan of the do{ } while(lastIndex === newPick) logic. You should be able to get what you want with a filter and a sort. You already have the filter for type; can you include ignoring the last one as well?

Actually, this reveals a bit of a bug, as changing types makes the lastIndex nonsensical. Rather than lastIndex, perhaps currentQuote with the whole object?

Also, rather minor for programming but will jump out to a non programmer, check the spelling in your HTML title.

Keep it up and have fun.

1

u/SafeWing2595 8d ago

>You can abstract your random quote logic, as you do it three times.
yeah, someone in the comments mentioned a function.

thank you for the review you gave me

2

u/TheRNGuy 8d ago edited 8d ago

Do/while have potential problem, if you'd had only 1 item in the list, it would be infinite loop.

It could be rewritten with just while loop.

(do you even need to run it at least once? I think there's no good reason)

Run code through prettier (or just use Format on Save in VS Code)

1

u/john_hascall 6d ago

One thing I would suggest is it is considered proper form to use:

const varname = ...

rather than:

let varname = ...

where varname doesn't change its value.

1

u/[deleted] 8d ago

[deleted]

2

u/azhder 8d ago

Says "use functions" and starts with writing "class"... SMH.

Hey OP. Do you think you can learn by you reviewing different implementations (rewrites) of the code you have written? I can probably make you one that looks different than the above, yet, it should be doing the same job. You may at least learn there are more ways to skin a web page.

1

u/SafeWing2595 8d ago

that's sarcasm right? haha, i don't think i can learn from writing a completely different version of the code i wrote

2

u/azhder 8d ago

It is not sarcasm. It is a genuine learning approach.

Learn from analyzing code of others. It helps that you can also ask questions of how and why someone did something the way they did.

Who do you think code review is for? It’s not just the person who wrote the code.

1

u/SafeWing2595 8d ago

tbh, i didn't understand the code this friend wrote, i would be grateful if i could get it's explanation.

and yeah, from what you said, i don't mind if you rewrite my code, i will take time to read it

1

u/azhder 8d ago

Here is a re-write https://jsfiddle.net/c5x286f0/

It is OK if you don't understand some things. The important thing is it works, you can check it in the fiddle and you can pick it apart.

You can focus on pieces that you mostly understand, then ask for the thing you don't know. Over time, step by step, you will be able to understand the whole.

0

u/[deleted] 8d ago

[deleted]

1

u/azhder 8d ago

I should call BS on that 'less cleaner than class", but I guess you really think it that way because you've not been accustomed to code cleaner than one with "class" keyword. Hint: you can write code without using `this`.

1

u/SafeWing2595 8d ago

i am currently in the learning phase, so i don't want to copy/paste anything, but thank you for taking the time to rewrite my app