1
\$\begingroup\$

Is there a neater way to write this code?

    var table, field, dbFieldEntries;
    var s = 'one.two';
    // The DB field is always the first parameter
    dbFieldEntries = s.split( '.' );
    if( dbFieldEntries.length === 1 ){
      field = dbEntryEntries[ 0 ];
    } else {
      table = dbEntryEntries[ 0 ];
      field = dbEntryEntries[ 1 ];
    }

Basically, s can be either something or something.other. If it's something, then table is undefined and field is something; it it's something.other, then table is something and field is other.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ It is a bit hard to review code like this because the code you are showing is just a short version of the code that you're actually using in a real project, which means that a lot of context has been stripped away. Unlike Stack Overflow, we prefer to look at real code instead of example code. Adding a description about what this code is supposed to do will greatly help, as would adding more code-context. \$\endgroup\$ Feb 2 '15 at 0:41
  • \$\begingroup\$ Did you downvote my question?!? 1) There was no point in showing anything more, since the code is fully self contained. I cut dump the whole of SimpleDbLayer if you like, it's not going to change the self contained chunk I want to improve 2) What the code does is written RIGHT BELOW the code. I thought that was really quite clear. The answer I see below, that I am about to accept, is indeed a fantastic response. Please get rid of the downvote since your observations were meaningless. \$\endgroup\$
    – Merc
    Feb 2 '15 at 4:45
  • \$\begingroup\$ For the record, I was not the one who downvoted your question. I can understand the one who did though because it is not entirely clear where the s value comes from, and if you would have revealed more about that then we could have suggested an entirely different approach which might have improved your code even more. \$\endgroup\$ Feb 2 '15 at 11:25
3
\$\begingroup\$

Assuming that field always gets the value of the last part of dbFieldEntries you could do:

field = dbEntryEntries[ dbFieldEntries.length - 1 ];
if ( dbFieldEntries.length > 1 ) {
  table = dbEntryEntries[ 0 ];
}

Besides this, s is a bad variable name.

\$\endgroup\$
1
  • \$\begingroup\$ Ah, this is exactly the kind of thing I was after. Thank you! This is so much better than the ugly stuff I had... \$\endgroup\$
    – Merc
    Feb 2 '15 at 4:46
2
\$\begingroup\$
field = dbFieldEntries.pop();
table = dbFieldEntries.length ? dbFieldEntries[0] : undefined;

... of course, that's working on the assumption that it's only possible to have one or two things seperated by a single ".", which your code definitely does assume.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.