Can someone help me fixing my code?

  1. c,d,e,f are 4 buttons(products)

requirement 1 :on clicking any of these

-> A div will render in which there are
a) - and + buttons and a count
b) an input field to enter description of product
(requirement 1 is completed)

-> on clicking + button a modal must appear which contain 2 buttons (1) repeat and (2) new

requirement 2 : on clicking repeat the count will increment by 1 (which means same product’s quantity increased)
on clicking new : a new div will render

data : the (1) “productDetails” object stores the quantity of product(each div)

Sorry, after a quick examination, I can’t understand what you are trying to do. I can’t figure out what the program is trying to do.

I would suggest creating a test app to just focus on the one little problem that you are having. Build the simplest program to do exactly what you are trying to do on that button press and let us know exactly what it is supposed to do and exactly what it is doing instead.

Being able to break problems into smaller problems is an important skill. And the more uncomfortable you are with something, the more often you should stop and test those little pieces, even if it means creating a whole tiny side project just to understand how that tiny piece works.

1 Like

First off, that is some pretty rough code, if you ask me.

Not sure i understand your question or the problem.

  1. What function?
  2. What is the expected behavior?
  3. What is the unexpected behavior?

There is one thing i don’t understand, And this might not be related to your question, i really can’t tell. Why is render incrementing totalCount. Shouldn’t it only be one time when pressing c (setting up first run) and then only one time for each time you press increment?

Like this:
(Edit: I have change the pre-increments, to post-increments because that just makes more sens to me.)

Summary
function showDiv(x) {
  // increment totalCount on first run
  state[x.dataset.name].totalCount++;
  ....
}

const render = x => {
  // only grap totalCount in render
  // let c = state[x].totalCount++;
  let c = state[x].totalCount;
  ....
}

function increment(x) {
  // increment totalCount
  let cat = x.dataset.name;
  state[cat].totalCount++;
  ....
}
1 Like

@lasjorg i know the code is a mess sorry for that
and btw you are absolutely right render should not increment it.
only increment and repeat functions should.
I am going to write the whole program from start :sweat_smile:

thank you @kevinSmith i will write the code again from scratch
and then also if i got problem i will ask for help.

@kevinSmith @lasjorg hey would you mind reading the question(topic) again? i did changes in project as well as topic.

Did you have a specific question?

Cart systems aren’t all that easy to get right. If you haven’t, I would suggest you try making a simple todo list first, it is similar but has more well-defined requirements. There are also a lot of tutorials on todo lists which might give you some ideas.

Based on your code, I will give you some general considerations:

  • Try not to use mutating global variables, store them in a structure or pass them around. It’s fine for const’s like DOM elements to be top-level, but they don’t have to be global, wrap your code in functions or objects.

  • If you do need mutating globals (you really shouldn’t except for testing), then don’t give them names like “i”. That is a name given to incrementers inside functions for indexing/looping, if I see a variable named “i” inside a function in a file on say line 356, I expect it to be local, not some global variable hanging around in the top of the file.

  • Use expected names for variables, if I see “e” I want it to be an event, not an element (you do use el for elements elsewhere in the code).

  • A render function should do what its name implies, and really nothing else. Ideally, a render function’s only side effect should be DOM manipulation. Don’t change internal data structures and don’t attach event listeners inside render functions, move that logic elsewhere.

  • Use event delegation to deal with handling events on dynamic DOM elements. Use a parent element that is always in the DOM and attach the listener to it, then do event delegation on its dynamically populated child elements.


I decided to have a play with your code. I guess I got a little carried away and rewrote everything. It’s far from ideal code, I have changed some things because I don’t really understand your functionality or the rationality behind them. It was really just for my own entertainment. Anyway, it’s poorly commented and has a bunch of logs in it, but maybe there is something you can glean from it.

1 Like

OMG buddy you are great hats off to you man
Sorry i was not able to convey what i need exactly
and my code is also very poor
btw TYSM for your suggestions and showing me how to write good code
you wrote whole code again that’s amazing :blush:
i made the page in react i will show you tomorrow (would like to know your reviews)