2
\$\begingroup\$

I'm working on a little project and although it all works, I think I might be doing it in an incorrect way. The front end consists of a simple form that gathers input data (make and color) and once submitted, it sends the input data to a POST endpoint in express via AJAX and gets filtered to return the items I need in a JSON file. Once the data posts, I use another AJAX call to get the filtered data back to my front end to display. Again this all works, but I'm posting and getting from the same route in express. Is this all good or can I accomplish this in a better manner?

Front end:

var carList = [];
form.addEventListener('submit', function(e) {
 e.preventDefault();
 var make = document.querySelector('.make').value.toLowerCase(),
 color = document.querySelector('.color').value.toLowerCase();
 $.ajax({
 method: 'POST',
 url: '/cars',
 data: {
 make: make,
 color: color
 },
 success: function(data) {}
 });
 $.ajax({
 method: 'GET',
 url: '/cars',
 success: function(data) {
 carList = data;
 populateList();
 }
 });
});

ExpressJS:

function readJSONFile(filename, callback) {
 fs.readFile(filename, (err, data) => {
 if(err) {
 callback(err);
 return;
 }
 try {
 callback(null, JSON.parse(data));
 } catch(exception) {
 callback(exception);
 }
 });
}
let make = '',
 color = '';
app.get('/cars', (req, res) => {
 let newList = [];
 readJSONFile('cars.json', (err, json) => {
 var items = json.cars.filter((item) => {
 if (item.type.toLowerCase() == make || item.color.toLowerCase() == color) {
 newList.push(item);
 }
 });
 if (err) throw err;
 res.send(newList);
 });
});
app.post('/cars', (req, res) => {
 make = req.body.make;
 color = req.body.color;
});
Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Apr 26, 2018 at 13:41
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$
  1. Avoid making multiple calls to express server, you can return filtered data from the POST call. Use single AJAX POST call. As your calls are being made parallel there is no way to know if the make and color would be set when you make the GET request.

  2. Use === to strict-compare values

  3. Array.filter() returns a filtered array so you can remove the newList. Look into Destructing

    readJSONFile('cars.json', (err, json) => {
     if (err) {
     res.status(400).json({
     error: err
     })
     }
     const items = json.cars
     .filter((item) =>
     item.type.toLowerCase() === make || item.color.toLowerCase() === color
     );
     res.send(items);
    });
    
  4. Convert the callback functions to promise and chain for readability. Promisify

Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
answered Apr 26, 2018 at 21:27
\$\endgroup\$
1
  • \$\begingroup\$ Hey thanks a lot for the reply. Your first point was my biggest concern there. I'll definitely check out the other ideas and use them. \$\endgroup\$ Commented Apr 27, 2018 at 13:23
1
\$\begingroup\$

I agree with Deep's answer - especially about just sending one POST request and having the POST endpoint return the data.

I noticed that the front end code appears to be selecting the elements that have the make and color values by class name:

var make = document.querySelector('.make').value.toLowerCase(),
 color = document.querySelector('.color').value.toLowerCase();
  1. It would be wise to use an id and fetch those elements using document.getElementById()

    var make = document.getElementById('makeInput').value.toLowerCase();
    
  2. It would be wise to cache those DOM references once the DOM is ready in constants and then reference those constants when the form is submitted.

document.addEventListener('DOMContentLoaded', function() {
 const make = document.getElementById('make');
 const color = document.getElementById('color');
 document.forms[0].addEventListener('submit', function(e) {
 console.log('make value: ', make.value, 'color value: ', color.value);
 e.preventDefault();
 });
});
<form>
 <div>Make: <input id="make" /></div>
 <div>Color: <input id="color" /></div>
 <input type="submit"/>
</form>

answered Apr 26, 2018 at 23:17
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the input here. I didn't even think about caching those items. \$\endgroup\$ Commented Apr 27, 2018 at 13:25

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.