dmfay / massive-js

A data mapper for Node.js and PostgreSQL.
2.49k stars 159 forks source link

casing best practices? #324

Closed jlippold closed 7 years ago

jlippold commented 7 years ago

I prefer to use camelCase for keys in javascript, like so:

{
    "firstName": "Bobby",
    "lastName": "Tables",
    "email": "bobby@aol.com"
}

Which I hope can map directly to a postgres table, of the same casing.

CREATE TABLE IF NOT EXISTS public.users (
    user_id SERIAL,
    firstName text NOT NULL,
    lastName text NOT NULL,
    email text UNIQUE NOT NULL,
    PRIMARY KEY(user_id)
);

If I try to use the above example in massive, it bombs.

    var userObject = {
        "firstName": "Bobby",
        "lastName": "Tables",
        "email": "bobby@aol.com"
    };
    db.users.insert(userObject, function(err, res) {
        // error is 
        // column "firstName" of relation "user" does not exist 
    });

I'd love to be able to populate my userObject and insert it back to postgres without reformatting the object coming out and in. Whats the recommended approach in massive?

If camelCase just isn't supported at all, is there a workaround? Maybe a global beforeExecute / afterExecute method i can hook into? If there were hooks, I could name fields in pg with underscores (first_name) and utilize these hooks to turn it into firstName globally.

Thanks for the help

dmfay commented 7 years ago

That's not Massive, it's Postgres. Look up the table in psql or whatever tool you use -- your field's actually firstname rather than firstName, since Postgres automatically fixes cases and then is case sensitive when you try to query it.

You can "quote" the field names in your create statement if you really want them to be cased like that in the database, but standard practice for Postgres is snake_case and given the sensitivity it's generally a good idea to go along with it. There are conversion modules like humps which would take some of the sting out of converting objects but it'll still be a bit slower to do that, of course.

jlippold commented 7 years ago

Thanks for the feedback @dmfay, if it's pg's best practice to use snake_case, I'll stick with that. But whats the recommended way to translate them?

I could have convertToCamel / convertToSnake, or use humps all over my code, but that is less than ideal. Is there's a global hook somewhere, or is there a better way?

dmfay commented 7 years ago

Massive is just a data mapper and uses pg to actually talk to the database; there's a pg-camelcase module that hooks into the driver to convert casing, but I don't think there's an easy way to use that with Massive since the pg dependency isn't exposed anywhere.

I just turn off camelcase in my jshintrc to shut it up and deal with it looking a little funny :)

jlippold commented 7 years ago

I ended up just writing a wrapper for massive so I can globally convert case. It's not the prettiest answer but it so far so good.

my wrapper is db.js like:

var massive = require('massive');
var humps = require('humps');
var env = process.env.NODE_ENV || "dev";
var config = require('../configs/massive')[env];
var db = massive.connectSync(config);
var dbWrapper = {};

//this only exposes methods I wrap manually, like findOne below
db.tables.forEach(function(dbTable) {
    var table = dbTable.name;

    dbWrapper[table] = {};
    dbWrapper[table].findOne = function(input, callback) {
        var snake = humps.decamelizeKeys(input);
        db[table].findOne(snake, function(err, output) {
            if (err) {
                return callback(err);
            }
            var camel = humps.camelizeKeys(output);
            return callback(err, camel);
        });
    }
    // needs more methods wrapped, eg..
    // dbWrapper[table].insert = function(input, callback) {}
});

module.exports = dbWrapper;

then i can call it like.

var db = require("./lib/db");
//inputs will be auto converted to snake
var params = {
    userName: user,
    deleted: false
};

db.users.findOne(params, function(err, user) {
    //user output will be converted to camel
});

¯(ツ)

robconery commented 7 years ago

I would strongly, urgently suggest you do not do this. It is a nightmare waiting to happen! Casing is a big deal and PG has a very strong opinion about it. As @dmfay suggested: Massive is wrapper and if you need to cast it to a something else then do it explicitly and make sure the code you write is justified in terms of existence. It might not seem like it now, but the code you're presenting here will likely be the #1 thing you support in the life time of the app.

If this is your preference (the casing): get over it. If this is the preference of your boss or organization, then make sure you have air cover.

jlippold commented 7 years ago

Hey @robconery, I understand that this might end up requiring long term support, but it feels cleaner to me than having redundant convert statements all over every route, or mixing 2 coding conventions. Are you suggesting I go snake_case across every object I expose to my api users? Should I start writing all of my JS with snakecase, when the standard for JS is camelCase?

I feel like this is really about choosing the lesser of two evils when none of them feel great.

robconery commented 7 years ago

I'm talking about a generalized conversion routine. Do as you will, of course, but in all the data access tools I've made, the naming of things (as well as type resolution) has always been a pain point. If casing is a big deal to you, I would suggest you deal with it on a "case by case" basis. Send the data into an object and have it load/resolve as you intend. Just my opinion...

jlippold commented 7 years ago

All good @robconery, thanks for the advice. Would you be open to accepting a pull request that adds pre & post execution hooks as I described in the first comment? Or do you feel that it's not beneficial for the tool?

jnystrom commented 7 years ago

@robconery do you let the underscored columns leak into your properties of your objects, or do you do a manual mapping from user_name to userName? This really is annoying...but I know it is not your fault... ;) What about table names? I assume if I have table named user_locations, massive is going to create a property name to query the table with the underscore, right (db.user_locations)? If that is correct, anyway to have Massive change table properties to camel case?

robconery commented 7 years ago

@jnystrom please don't take this the wrong way, but writing code based on annoyances isn't something I want to do :). I hate to put it this way, but maybe just get over it :). Honestly, I don't mean to be shit about it ... it's just an underscore. Code == work I have to do by way of support. The less the better :).

jnystrom commented 7 years ago

@robconery just looking for advice on how you would do it, not trying to be a pain in the ass.

robconery commented 7 years ago

@jnystrom Oops - didn't answer your question sorry :). Yes, I just let the underscores in. I quite like it because I know which table I'm using, or which function/view.