Simple calculator in Java I've been learning Java for about 3 weeks now, and I was hoping someone could check over my code for me, and let me know how to improve.

I am aware that the maths class could be removed (I could've typed answer = inputA * inputB etc. within the switch statement) but this is just because I wanted to practice using multiple classes.

import java.util.Scanner;

public class Calculator {

public static void main(String[] args) {

    Scanner input = new Scanner(System.in);

    Maths Maths = new Maths();

    double answer = 0;
    double inputA, inputB;
    char operator;
    boolean done = false;

     while (done == false) {
        System.out.print("Please enter your sum: ");

        inputA = input.nextDouble();
        operator = input.next().charAt(0);
        inputB = input.nextDouble();

        switch (operator) {
            case '+': answer = Maths.add(inputA, inputB);
                      break;
            case '-': answer = Maths.subtract(inputA, inputB);
                      break;
            case '*': answer = Maths.multiply(inputA, inputB);
                      break;
            case '/': answer = Maths.divide(inputA, inputB);
                      break;
            case '^': answer = Maths.power(inputA, inputB);
                      break;
        }

            System.out.println(answer);
    }

    input.close();

  }

}

And my second class...

public class Maths {

    double add(double a, double b) {
        double answer = a+b;
        return answer;
    }

    double subtract(double a, double b) {
        double answer = a-b;
        return answer;
    }

    double multiply(double a, double b) {
        double answer = a*b;
        return answer;
    }

    double divide(double a, double b) {
        double answer = a/b;
        return answer;
    }

    double power(double a, double b){
        double answer =a;

        for (int x=2; x<=b; x++){
            answer *= a;
        }

        return answer;
    }

}

power should probably be using Math.pow().  I'm picturing how slow your code would be at calculating 1.00000015000000.  Or how inaccurate it'll be when someone wants to raise to a non-integer power.  If those are not intended to be allowed, then passing doubles all over the place seems odd.  But it'd be better to check beforehand and throw an error than return a blatantly incorrect result, in my opinion.
You never set done to true, and thus have an infinite loop; take out the flag and say while (true) or for (;;), so that that's apparent.  Don't add the flag back til you're also adding the code that sets it.
The fact that you use objects doesn't make your code object-oriented.  How you use them is what makes the difference.  In this case, you're using classes as namespaces for functions, which is not very object-oriented at all.

In your case, Maths is entirely useless; everything it does would be better done in your own code.  This is an example of gratuitous use of objects.  If you wanted to practice your OOP, you might want to think about how you'd get rid of that switch statement.  Hint:

public interface BinaryOperation {
    public double resultFor(double left, double right);
}

class Multiplication implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left * right;
    }
}

class Addition implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left + right;
    }
}

private Map<Character, BinaryOperation> operations;

...

operations.put('*', new Multiplication());
operations.put('+', new Addition());

...

BinaryOperation op = operations.get(operator);
if (op != null) {
    answer = op.resultFor(inputA, inputB);
}
else {
    System.out.println("Error: Unknown operator");
]


Calculating the distance between one point, and many others In my program, I have entities that I call "blobs", because they have a blobby shape. Blobs are polygons. If I have two blobs, then their information array would look like:

>>> blobs
np.array([ [ [x1, y1], [x2, y2], [x3, y3] ], [ [x1, y1], [x2, y2], [x3, y3] ] ])

Each entry along axis 0 represents one blob. Each entry along axis 2 represents the x,y coordinates of the vertex of a blob. In the above case, there are 2 blobs, each with three vertices.

There could be other information in each

We can assume that all blobs will always have the same number of vertices.

I sometimes need to find which blobs are within a certain distance from another blob.

Here are the two functions I use:

def returnBlobIndicesOfRelevantBlobs(minimumDistanceFromFocusVertex, blobs, focusVertexBlobIndex, focusVertexIndex):
    focusVertexCoords = blobs[focusVertexBlobIndex, focusVertexIndex, :2]

    # get the coordinate of the zeroth vertex for all blobs
    vertexCoordsForAllRelevantBlobs = blobs[:, 0, :2]

    distanceVectors = (vertexCoordsForAllRelevantBlobs - focusVertexCoords)
    distances = np.sqrt(np.einsum('ij,ij->i', distanceVectors, distanceVectors))

    mask = (distances <= minimumDistanceFromFocusVertex)
    return np.arange(len(blobs))[mask]

def returnBlobVertexIndicesOfRelevantBlobs(minimumDistanceFromFocusVertex, blobs, focusVertexBlobIndex, focusVertexIndex):
    # trying to reduce the number of calculations by only executing calculations on the closest blobs
    relevantBlobIndices = returnBlobIndicesOfRelevantBlobs(4*minimumDistanceFromFocusVertex, blobs, focusVertexBlobIndex, focusVertexIndex)

    # can the generation of this list be improved perhaps?
    relevantBlobVertexIndices = np.array(list(itertools.product(relevantBlobIndices, range(len(blobs[0])))))

    focusVertexCoords = blobs[focusVertexBlobIndex, focusVertexIndex, :2]

    # can the generation of this list be improved perhaps?
    blobVertexCoords = np.array([blobs[x[0], x[1], :2] for x in relevantBlobVertexIndices ])

    distanceVectors = (blobVertexCoords - focusVertexCoords )
    distances = np.sqrt(np.einsum('ij,ij->i', distanceVectors, distanceVectors))

    mask = (distances <= minimumDistanceFromFocusVertex)
    return relevantBlobVertexIndices[mask]

What improvements could I make?

Your existing algorithm is a bit perplexing to me.  In particular, the returnBlobIndicesOfRelevantBlobs doesn't really do much except cause the answer to occasionally be wrong.  For example here's what I'm using to drive your code:

from timeit import default_timer as timer
# your code goes here

blobs = np.array([
    [ [0,0],  [0,0], [0,0] ],
    [ [12,2], [9,0], [0,0] ],
    [ [-1,1], [0,0], [0,3] ]
])

start = timer()
r = 2
ans = returnBlobVertexIndicesOfRelevantBlobs(r, blobs, 0, 0)
elapsed_time = (timer() - start)
print "Found {0} in {1} s.".format(len(ans), elapsed_time)

For efficiency, you can eliminate the use the np.sqrt by simply using the square of minimumDistanceFromFocusVertex (my those are long names!) which you would only need to calculate once.

Your use of 4*minimumDistanceFromFocusVertex seems a bit arbitrary and probably incorrect.  For this particular example, for instance, it causes the second blob (with first coordinate [12,2]) to be removed from further consideration even though its third coordinate is within the target range.

If you're using Python 2.7 (as I am) you should use xrange instead of range.  The difference is that range actually allocates memory and populates an array while range does not.  (In Python 3.3, range now does what xrange did in Python 2.7, and xrange is deprecated.)

Finally, while there may be good reason for your application, it's not at all clear to me why one would create an array of indices as the desired final answer.  If that's really what you wanted, then that's fine, but if you find that you're having to do some additional transformation before using it, consider ways in which that this code could deliver the results is a more directly usable form.

As a final check, and speed test you can use this:

# first create a pseudo-random population of blobs
np.random.seed(7)
r=10000
n=4000000
blobs = np.random.random_integers(-r, r, (n,3,2))
# set the first vertex at the origin
blobs[0,0] = [0,0]

start = timer()
ans = returnBlobVertexIndicesOfRelevantBlobs(r, blobs, 0, 0)
elapsed_time = (timer() - start)
print "Found {0} in {1} s.".format(len(ans), elapsed_time)
print "The value of pi is approximately {}".format((4.0*len(ans))/(3.0*n))

As a bonus, you can use this to calculate an approximate value of \$\pi\$ which serves as a useful sanity check.  That works because the code above places 3 random points per \$n\$ blobs in a square that measures \$2r\$ on each side for an area of \$4r^2\$.  The code then counts the number of points that are within \$r\$ units of the center, which of course describes an inscribed circle with area \$\pi r^2\$.  Since all of the points are placed with uniform distribution, if \$a\$ is the number of points within the circle, we know that
$$\displaystyle \frac{a}{3n} \approx \frac{\pi r^2}{4 r^2} $$
or in other words, that the odds that each point lands in the circle within the square is proportional to the areas of the circle and square.  So,
$$\displaystyle \frac{4a}{3n} \approx \pi$$

When I run the program on my machine with the values shown above, I get:

Found 9424443 in 41.8755390644 s.
The value of pi is approximately 3.141481

I'm sure there are ways to speed up these calculations, which I may have time to pursue later.


How May I extract this ternary operator in independent statement Somehow this code looks messy to me:

 var results = query.GroupBy(x => x.ProductStatusId == ProductStatus.Sold || x.ProductStatusId == ProductStatus.OnHold ? "Delivered" :
                                x.ProductStatusId == ProductStatus.Shiped ? "Shiped" :
                                x.ProductStatusId == ProductStatus.NotAvailable ? "NotAvailable" :
                                x.ProductStatusId == ProductStatus.Paid ? "Paid" : "New").Where(g => g.Key != "New")

Is it possible to extract this ternary operator in independent statement.

Thanks!

Cheers

You could do query.GroupBy(MyTernaryWrapperMethod) and declare that method as

string MyTernaryWrapperMethod(xVariableType x)
{
    if(x.ProductStatusId == ProductStatus.Sold || x.ProductStatusId == ProductStatus.OnHold) return "Delivered";

    if(x.ProductStatusId == ProductStatus.Shiped) return "Shiped";

    if(x.ProductStatusId == ProductStatus.NotAvailable) return "NotAvailable";

    if(x.ProductStatusId == ProductStatus.Paid) return "Paid";

    return "New"
}

But, that method seems to return something being an actual property of x. So maybe you should add a new method to x's type. Also it would be better to use swicth/case instead of those ifs:

public string ProductStatusName()
{
    switch (ProductStatusId)
    {
        case ProductStatus.Sold:
        case ProductStatus.OnHold: return "Delivered";
        case ProductStatus.Shiped: return "Shiped";
        case ProductStatus.NotAvailable: return "NotAvailable";
        case ProductStatus.Paid: return "Paid";
        default: return "New"
    }
}

And then you do:

var results = query.GroupBy(x => x.ProductStatusName()).Where(g => g.Key != "New")


Finding the sum of all the multiples of 3 or 5 below any given number I wrote the following simple function in JavaScript that finds the sum of all the multiples of 3 or 5 below a value, given from a text-box.

I have used \$\dfrac{n}{2(a_1 + a_n)}\$ (sum of arithmetic sequence).

I was wondering if there is a way to shorten or otherwise improve this code.

function sum_of_all_mul_3_5(){
    var value=parseInt(document.getElementById("input").value);
    var result=(((Math.floor((value-1)/3))*(3+((Math.floor((value-1)/3))*3)))/2)+(((Math.floor((value-1)/5))*(5+((Math.floor((value-1)/5))*5)))/2)-(((Math.floor((value-1)/15))*(15+((Math.floor((value-1)/15))*15)))/2);
    document.getElementById("output").value=result ;
    }

I'm going to take by "shorten", you mean "remove this duplication", in which case the answer is yes. Extract an SumSequence function. Give things some breathing space while you're at it.

function sum_of_all_mul_3_5(){
    var value=parseInt(document.getElementById("input").value);
    var result=sumSequence(value, 3) + sumSequence(value, 5) - sumSequence(value, 15);
    document.getElementById("output").value=result ;
    }

function sumSequence(value, x){
    return (Math.floor((value - 1) / x) * (x + ((Math.floor(value - 1) / x) * x))) / 2);
    }

For the record though, using the arithmetic sum of the sequence is a very elegant solution to this problem though. I know of no more efficient way to do it, but a comment explaining what's going on here would be beneficial to any future maintainer. You might even want to go the extra mile and drop a link to wikipedia in that comment.


Javascript class+call base class? I'm sure this can be done in less lines and in a more clean way?

function BaseClass() {
  BaseClass.prototype.talk = function () {
    alert("I'm BaseClass");
  }
}

function MyClass() {
  BaseClass.call(this);
}
MyClass.prototype = new BaseClass();
MyClass.base = {};
MyClass.base.talk = MyClass.prototype.talk;
MyClass.prototype.talk = function () {
  alert("I'm MyClass");
  MyClass.base.talk();
}

var a = new MyClass();
a.talk();

function BaseClass () {}

BaseClass.prototype.talk = function () {
    alert("I'm BaseClass");
}

function MyClass() {
    BaseClass.call(this);
}

MyClass.prototype = new BaseClass();
MyClass.base = BaseClass.prototype;

MyClass.prototype.talk = function () {
    alert("I'm MyClass");
    MyClass.base.talk.apply(this, arguments);
}

var a = new MyClass();
a.talk();

To make all this easier on yourself, don't re-invent the wheel. Look into John Resig's tiny inheritance library: Simple JavaScript Inheritance.


Google Apps Script for finding empty days on multiple calendars I need help with a simple Google Apps Script I've written. The script finds days on which no all-day events have been scheduled, across multiple calendars.

I'm fairly certain the main issue is the fact that I'm making an API call to get all owned calendars, then I'm making 5 API calls on each of these calendars to retrieve each day's (Mon - Fri) events.

Currently, it takes about 60 seconds to run on an account with 11 owned calendars. Is there anything that can be done to optimize?

var one_day = 86400000; //24 * 60 * 60 * 1000
var days = ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday'];
var startDay = nextMonday(new Date());

// Script-as-app template.
function doGet() {
  var app = UiApp.createApplication();

  var button = app.createButton('Find Blanks').setId('button');
  var loader = app.createImage('http://preloaders.net/preloaders/496/Flip%20Flop.gif').setVisible(false).setId('loader');
  var blanks = app.createHTML().setId('blanksArea').setHTML('Click button to find calendar blanks for week of ' + startDay);

  app.add(button);
  app.add(app.createHTML('
'));
  app.add(loader);
  app.add(blanks);

  var clientHandler = app.createClientHandler().forEventSource().setEnabled(false).forTargets(loader).setVisible(true).forTargets(blanks).setHTML('Processing for week of ' + startDay);
  var handler = app.createServerHandler('serverHandler');

  button.addClickHandler(clientHandler);
  button.addClickHandler(handler);

  return app;
}

function serverHandler(e) {
  var app = UiApp.getActiveApplication();

  var button = app.getElementById('button');
  var blanks = app.getElementById('blanksArea');
  var calendarsWithBlanks = getCalendarsWithBlanks();

  blanks.setHTML(calendarsWithBlanks.length ? calendarsWithBlanks.join('
') : 'There are no blanks');

  app.getElementById('loader').setVisible(false);
  button.setEnabled(true);

  app.close();
  return app;
}

function getCalendarsWithBlanks()
{
  var calendars = CalendarApp.getAllOwnedCalendars();
  var calendarsWithBlanks = [];
  for(var i = 0; i < calendars.length; i++)
  {
    var calendar = calendars[i];
    var calendarName = calendar.getName();
    var blankDays = checkCalendarForBlanks(calendar, startDay);
    if(blankDays.length)
    {
      calendarsWithBlanks.push(calendarName + ' has ' + blankDays.join(', ') + ' blank')
    }
  }
  return calendarsWithBlanks;
}

function checkCalendarForBlanks(calendar, startDay)
{
  var blankDays = [];
  for(var d = 0; d < 5; d++)
  {
    var currDay = new Date(startDay.getTime() + (d * one_day));
    var events = getEventsForDay(calendar, currDay);
    var foundAllDayEvent = false;
    for(var e = 0; e < events.length; e++)
    {
      if(events[e].isAllDayEvent())
      {
        foundAllDayEvent = true;
        break;
      }
    }
    if(!foundAllDayEvent)
    {
      blankDays.push(days[currDay.getDay()]);
    }
  }
  return blankDays;
}

function nextMonday(date)
{
  var today = date;
  today.setUTCHours(0);
  today.setMinutes(0);
  today.setSeconds(0);
  today.setMilliseconds(0);
  var day = today.getDay();
  var offset = 8 - day
  var next_monday = new Date(today.getFullYear(), today.getMonth(), today.getDate()+offset);
  return next_monday;
}

function getEventsForDay(calendar, date)
{
  /* !!!KLUDGE ALERT!!!
   * This is a kludge. The API method getEventsForDay is wonky: http://stackoverflow.com/questions/11870304/google-apps-script-geteventsforday-returns-invalid-recurring-events
   */

  var startOfDay = new Date(date.getTime());
  startOfDay.setUTCHours(0);
  startOfDay.setMinutes(0);
  startOfDay.setSeconds(0);
  startOfDay.setMilliseconds(0);
  var endOfDay = new Date(startOfDay.getTime() + one_day);

  return calendar.getEvents(startOfDay, endOfDay)
}

I flipped your logic a bit in the example below.  I only do one API call per calendar and then do all of the processing in the client.  It seems a bit faster for me, but I'm not seeing the 60 second processing time that you are.  So your results my vary.

var one_day = 86400000; //24 * 60 * 60 * 1000
var days = ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday'];
var startDay = nextMonday(new Date());

// Script-as-app template.
function doGet() {
  var app = UiApp.createApplication();

  var button = app.createButton('Find Blanks').setId('button');
  var loader = app.createImage('http://preloaders.net/preloaders/496/Flip%20Flop.gif').setVisible(false).setId('loader');
  var blanks = app.createHTML().setId('blanksArea').setHTML('Click button to find calendar blanks for week of ' + startDay);

  app.add(button);
  app.add(app.createHTML('
'));
  app.add(loader);
  app.add(blanks);

  var clientHandler = app.createClientHandler().forEventSource().setEnabled(false).forTargets(loader).setVisible(true).forTargets(blanks).setHTML('Processing for week of ' + startDay);
  var handler = app.createServerHandler('serverHandler');

  button.addClickHandler(clientHandler);
  button.addClickHandler(handler);

  return app;
}

function serverHandler(e) {
  var app = UiApp.getActiveApplication();

  var button = app.getElementById('button');
  var blanks = app.getElementById('blanksArea');
  var calendarsWithBlanks = getCalendarsWithBlanks();

  blanks.setHTML(calendarsWithBlanks.length ? calendarsWithBlanks.join('
') : 'There are no blanks');

  app.getElementById('loader').setVisible(false);
  button.setEnabled(true);

  app.close();
  return app;
}

function test_getCalendarsWithBlanks() {
  getCalendarsWithBlanks().forEach(function(calendarResult) {
    Logger.log(calendarResult);
  });
 }

function getCalendarsWithBlanks()
{
  var calendars = CalendarApp.getAllOwnedCalendars();
  var calendarsWithBlanks = [];
  for(var i = 0; i < calendars.length; i++)
  {
    var calendar = calendars[i];
    var calendarName = calendar.getName();
    var blankDays = checkCalendarForBlanks(calendar, startDay);
    if(blankDays.length )
    {
      calendarsWithBlanks.push(calendarName + ' has ' + blankDays.join(', ') + ' blank')
    }
  }
  return calendarsWithBlanks;
}

function checkCalendarForBlanks(calendar, startDay)
{
  var blankDays = [],
      allDayEventsPerDay = [0,0,0,0,0,0,0],
      endDate = new Date(startDay.getTime() + (4 * one_day)),
        // get all events for the next work week
      allEventsForNextWorkWeek = calendar.getEvents(startDay, endDate);

  //count number of all day events on each day
  allEventsForNextWorkWeek.forEach(function(event){
    if(event.isAllDayEvent()) {
      //count each day the event is on
      determineDaysOnWhichEventTakesPlace(event,startDay,endDate).forEach(function(eventDate){
        allDayEventsPerDay[eventDate.getDay()]++;
      });
    }
  });

  //figure out which ones have 0 (ie blank days)
  allDayEventsPerDay.forEach(function(count, offset){
    if(count === 0) {
      blankDays.push(days[offset]);
    }
  });

  return blankDays;
}

function determineDaysOnWhichEventTakesPlace(event, lowerLimit, upperLimit) {
  var startDate = new Date(Math.max(event.getAllDayStartDate().getTime(), lowerLimit.getTime())),
      endDate = new Date(Math.min(event.getAllDayEndDate().getTime(), upperLimit.getTime())),
      dates =[],
      eventDate;

    for(eventDate = startDate; eventDate< endDate; eventDate = new Date(eventDate.getTime() + (one_day))) {
      dates.push(eventDate);
    }
  return dates;
}

function nextMonday(date)
{
  var today = date;
  today.setUTCHours(0);
  today.setMinutes(0);
  today.setSeconds(0);
  today.setMilliseconds(0);
  var day = today.getDay();
  var offset = 8 - day
  var next_monday = new Date(today.getFullYear(), today.getMonth(), today.getDate()+offset);
  return next_monday;
}


Element selectors for a RepeatButtonView I have a large application with a wide-reaching amount of views and sub-views. I have a need to decorate DOM elements with identifiers so that my framework of choice can bind views to the DOM elements.

In attempting to choose a robust convention for naming these selectors, I've encountered a few problems:

id selectors: it's difficult to maintain unique ID selectors across a large amount of views. Prefixing IDs can help alleviate this issue, but is overly verbose.
class selectors: normally classes are used just for CSS decoration. This CSS best practices guide suggests prefixing JavaScript-only CSS classes with js- to indicate there is no CSS decoration. This helps alleviate the issue, but can lead to confusing/error-prone code due to a mixture of js- and unprefixed classes.
data selectors: I can't see any reason to not use these for this purpose aside from being very non-standard. I haven't seen this used in any production code I've looked at.

Here are examples of the three scenarios above using Backbone.Marionette. The examples should be simple enough for the framework to not matter, though.

Does anyone have additional experience/opinions to contribute to the matter? Anything incorrect about the data selector route?

id selector

Too verbose and doesn't work on collections:

var RepeatButtonView = Marionette.ItemView.extend({
    id: 'repeatButton',
    template: _.template('<div id=<%= viewId %>-repeatSingleIcon>
<div id=<%= viewId %>-repeatAllIcon>
'),
    templateHelpers: function(){
        return {
            viewId: this.id
        };
    },

    ui: function(){
        return {
           repeatSingleIcon: '#' + this.id + '-repeatSingleIcon',
           repeatAllIcon: '#' + this.id + '-repeatAllIcon'
        };
    }
});

class selector

use js- prefix to indicate class is only used for JavaScript. Potentially weird UI declaration when some classes have CSS.

var RepeatButtonView = Marionette.ItemView.extend({
    id: 'repeatButton',
    template: _.template('

'),
    ui: {
        repeatSingleIcon: '.js-repeatSingleIcon',
        repeatAllIcon: '.repeatAllIcon'
    }
});

.repeatAllIcon {
    pointer-events: all;
}

data selector

potentially confusing due to rarity of implementation

var RepeatButtonView = Marionette.ItemView.extend({
    id: 'repeatButton',
    template: _.template('

'),
    ui: {
        repeatSingleIcon: '[data-ui*=repeatSingleIcon]',
        repeatAllIcon: '[data-ui*=repeatAllIcon]'
    }
});

Using data-* attributes is actually part of the HTML5 standard.

Custom data attributes are intended to store custom data private to the page or application, for which there are no more appropriate attributes or elements.
These attributes are not intended for use by software that is independent of the site that uses the attributes.

(emphasis mine)
In my opinion, I like to keep behavior separate from style, so I lean towards using data-* attributes. A problem arises when you attach CSS styles to those custom attributes, though I must admit that some styling is OK (for instance, showing or hiding elements). If you want a data-ui=child element styled a certain way, add a class attribute to it to maintain your separation of style and behavior.
My only piece of advice when using data attributes is to "namespace" them such that you have data-ui and more specific or related attributes are data-ui-something where each data attribute your view framework uses is prefixed with data-ui. This helps prevent naming collisions with other people's code.


Reversing strings using recursion I'm trying to solve exercises where I am required to use recursion. I wrote this:

static String reverse(String word){
    if(word.length()<2){
        return word;
    }
    else{
        int index = word.length() - 1;
        return word.charAt(index) + reverse(word.substring(0, index));
    }
}

But the answer for that question was the following code:

static String reverseRecursively(String str) {
    //base case to handle one char string and empty string
    if (str.length() < 2) {
        return str;
    }

    return reverseRecursively(str.substring(1)) + str.charAt(0);

}

And now let's suppose that you are a recruiter and got the first solution. Do you accept it? If not, why? What should I do better?

In production Java code, you would never reverse a string using recursion, primarily due to efficiency concerns, and also because new StringBuilder(str).reverse().toString() is better.  Therefore, this question should be treated as an academic exercise — a quick test to weed out candidates who don't understand recursion at all.  I don't think that an optimal solution is required.

Basically, your solution is good enough.  That said…

I prefer your reverse() to reverseRecursively(), since I consider the recursion to be an implementation detail that doesn't need to be reflected in the name.
str is a more appropriate parameter name, since the function doesn't care whether the string is a word or not.
The way you sliced the string is less elegant than in the second solution.
I find your brace style slightly irksome.  (Normally I wouldn't say anything about it, but given how little code I have been given to evaluate, I'm judging you on that.)  Try to stick to the obsolete official Java style guide or Google's Java style guide, which is a close derivative of it.


Taking a node in rootA and finding a clone node in other tree I was asked this as an interview question. I was also asked to assume that there is a function which can compare and say two nodes are identical. I would like to implement that function as well. I'm not sure what's a sure shot way of saying that in pure JavaScript.

    O         O
   / \       / \
  O   O     O   O
     /|\       /|\
    O O O     O O O

  Assume two DOM trees with roots: rootA and rootB, the rootB tree
  being a clone of the rootA tree.  Write a function which takes a node
  in rootA and finds a clone node in the other tree.

I believe there is a better solution than recursion.

jsFiddle

function walkTheDom(root, cb) {
    if (!root) {
        return;
    }
    var r = root;
    if (cb (r)) {
        foundNode = r;
        return;
    }
    r = r.firstChild;

    while (r) {
        walkTheDom(r, cb);
        r = r.nextSibling;
    }

}

var compare = function (el) {
    //return true if nodes are equal
    //false if they're not
}

function findNode(node, rootB) {
    var foundNode = '';
    walkTheDom(rootB, compare);
    if (foundNode) {
        return foundNode
    }
}

It looks like the solution solves the wrong problem: the rootA is completely ignored, as well as the fact that the trees are actually DOM. In a DOM structure it is safe to assume that the node has an access to its parent and siblings. So, given a node in rootA tree, you may trace its path to the root, and then descend the same path down from rootB. Since they are clones, such descend will get you to the node in question. I don't see the need (other than unit test purposes) to compare nodes.


Updates or creates an entity based on if a value is present in an Optional I need to get car info from a 3rd party web service and persist the data in my application DB. If my DB already has the car, I only update property values that may have changed. Otherwise, I create and save the car:
@Transactional //Spring Transaction
void saveCars() {

   //get CarInfo from a webservice
   List<CarInfo> carInfos = getCarInfo();

   // now get each Car from my application DB
   // that correspond to each CarInfo.
   Set<Car> cars = mySpringCarRepo.findByCarInfoIdIn(
       carInfos.stream().map(c -> c.getId())
           .collect(Collectors.toSet());

   for (CarInfo carInfo : carInfos) {

      // If a car for the carInfo
      // is already in my application
      // just update it if some carInfo has changed.
      // If a car is not there, create and save it to DB.
      Car car = cars
                 .stream()
                 .filter(c -> c.getCarInfoId().equals(carInfo.getId())
                 .findAny()
                 .map(c -> updateCar(carInfo, car))
                 .orElseGet(() -> createCar(carInfo);

       mySpringCarRepo.save(car);
   }
}

I have the following questions / concerns about this code:

Is this an inappropriate use of Optional's map method? I'm not mapping to a new object but rather just returning the same car with some properties potentially updated.
The save method is really only required to be called for cars created in the orElseGet. Due to being in a transaction, the updates to the car will be persisted without the save call. It seems redundant and perhaps confusing to call save.

Effective Java 3rd edition states that many uses of isPresent can profitably be replaced by other Optional methods. So that has led me to the path of using map & orElseGet and the fact that Java 8 does not have a ifPresentOrElse method on Optional.

Regarding your questions

Is this an inappropriate use of Optional's map method? I'm not mapping
to a new object but rather just returning the same car with some
properties potentially updated.

I think it's appropriate because it might return an updated object. Unfortunately (as you said) Java 8 doesn't have ifPresentOrElse.

The save method is really only required to be called for cars created
in the orElseGet.

You can change the orElseGet with orElseGet(() -> mySpringCarRepo.save(createCar(carInfo))) and remove the final save to the db.
Performances
The complexity of the method is O(c*ci) where c is the number of cars in your DB and ci is the number of CarInfo.
If you store the Cars in a Map instead of a Set you can improve it to O(ci).
void saveCars() {
    // get CarInfo from a webservice
    List<CarInfo> carInfos = getCarInfo();
    // Get cars from DB that correspond to each CarInfo
    Map<Long,Car> cars = mySpringCarRepo
            .findByCarInfoIdIn( getCarInfoIds(carInfos) )
            .stream()
            .collect( Collectors.toMap(Car::getCarInfoId,Function.identity()));

    for (CarInfo carInfo : carInfos) {
        // Returns Optional<Car>
        Optional.ofNullable(cars.get(carInfo.getId()))
        // Update car if already exists
        .map(c -> updateCar(carInfo, c))
        // Save car otherwise
        .orElseGet(() -> mySpringCarRepo.save(createCar(carInfo)));
    }
}

Or if you don't mind to query the db for each CarInfo, than the method is shorter:
void saveCars() {
    getCarInfo().stream()
        .forEach(carInfo ->
                    // returns Optional<Car>
                    mySpringCarRepo.findByCarInfoId(carInfo.getId())
                    // Update car if already exists
                    .map(car -> updateCar(carInfo, car))
                    // Save car otherwise
                    .orElseGet(() -> mySpringCarRepo.save(createCar(carInfo))));
}

Note 1: I haven't fully tested the code, it's just to give you an idea how to improve it.
Note 2: the performance gain might be irrelevant due to the DB and network latency, but with the second approach you should save some memory space.
Naming
The method name saveCars is not very appropriate, a better name might be updateCars or syncCars.


Permutation-stream based n-queens solution Following is a very crisp solution for solving n-queens problem in Racket programming language. The method is to get all permutations (so that no 2 queens are in same line) and to check these for any of them being on diagonals:

#lang racket

; FOLLOWING FN RETURNS TRUE IF QUEENS ARE ON DIAGONALS:
(define (check-diagonals bd)
  (for/or ((r1 (length bd)))
    (for/or ((r2 (range (add1 r1) (length bd))))
      (= (abs (- r1 r2))
         (abs(- (list-ref bd r1)
                (list-ref bd r2)))))))

; SET BOARD SIZE:
(define N 8)

; 3-LINE SEARCH LOOP:
(for ((brd (in-permutations (range N))))
  (when (not (check-diagonals brd))
    (displayln brd)))

The in-permutations function returns a stream sending one permutation at a time, so it does not find all permutations in advance.

Can this code be further improved to increase its speed for larger numbers, especially by parallelization or concurrency? Thanks for your help.

(PS: I have also posted this at https://stackoverflow.com/questions/46144576/parallel-running-of-racket-code)

Variable Names

There is little reason not to give the variables clear meaningful names. Using abbreviations leads to using bd in check-diagonals and brd in the search loop. Calling the board board makes the code clearer. Similarly, the time a reader spends figuring out that r1 and r2 refer to rows is wasted time for every reader. It saved the programmer four keypresses once.

The size of the board is a natural number. The traditional notation for the Natural Numbers is n not N. It's why it's the 'n-queens' problem not the 'N-queens' problem.

Concurrency and Parallelization

Concurrency covers all problems where the execution can be logically simultaneous whether or not the implementation runs in parallel or not. Most concurrent processes do not run entirely in parallel. Parallelization is a way of executing a concurrent process. All concurrent processes are not run in parallel. All parallel processes are logically concurrent.

The n-queens problem

The solution space of n-queens problem is non-polynomial. The size is n!. Parallelization with p nodes still leaves the solution space non-polynomial, approximately n!/p. A solution that would take a 100 million years still takes an impractical time when spread across 1,000,000 nodes.

What makes the n-queens problem interesting is that it is non-polynomial and that it has symmetries that allow for the search space to be pruned.  For example, many configrations are isomorphically related to  other configurations by board rotation and mirroring.

Optimizing solutions to NP problems

There is no general algorithm for quickly solving non-polynomial problems. That's what makes them interesting/hard depending on one's perspective. Speeding up the code to a specific problem requires taking advantage of the problem specifics.

In the case of n-queens, pruning the search space of isomorphic non-solutions each time a non-solution is found. Optimization comes from writing code that coming across the the n=3 non-solution:

  a b c
1 Q Q
2
3

prunes the search space of isomorphic configurations such as:

  a b c
1   Q Q
2
3

and

  a b c
1     Q
2     Q
3

Other Strategies

An empty board has zero probability of being a solution. This is lower than a board with a queen randomly placed in each column. Though the probability of random placement producing a solution is slight, it is better than an empty board.

Gradient Descent Starting with a randomly populated board, an estimate of the distance from a solution is made. [1] Next each alternative position for each queen is examined and an estimate of the distance from a solution is made for each resulting configuration. The one move that results in minimizing the distance from the solution is made and the process repeats.

When no improvement is possible the solution may be abandoned [2] and a new random board generated. Generating and examining random boards is parallizable and typically done because the number of boards examined is often large.

[1]: Choosing how to measure the distance is art informed by science, not pure science except in so far as recognizing that the problem of measuring is still in NP.

[2]: More complex approaches may not always chose the configuration closest to a solution and/or backtrack when no improvement is possible. Again, optimization is based on understanding the particulars of the problem.


Design of tree data structure I am practicing for technical interviews and I am encountering a lot of problems relating to the tree data structure. I have gone through other posts that discuss about the tree data structure and come up with the class definition and method signatures for the some of the operations as below:

class Tree {
    struct Node {
        int data;
        Node *left;
        Node *right;
        Node(int d): data(d), left(nullptr), right(nullptr) {}
    };
    Node *root_;
    Tree(Node *r): root_(r) {};
public:
    Tree(): root_(nullptr) {};
    ~Tree();
    void deserializeTree(std::string serializedTree);
    Tree::Node* deserializeTreeHelper(Tree::Node *root, std::string serializedTree, int *start);
    void printTreeHelper(Node *root);
    void printTree();
    void mirrorTree();
};

Since a lot of operations related to trees can be modeled as recursive functions, with my above class definition, I seem to end up having a method that can be invoked on the tree object and another helper method that does the recursive operation.

My reasoning in having a helper method is because recursive tree operations like the one below needs to return the root of a sub tree and root is of type Tree::Node which is private to the Tree. Hence I can't have the parent method (the method that invokes the recursive helper method) returning Tree::Node.

void
Tree::deserializeTree(std::string serializedTree) {
    if (serializedTree.length() <= 0 || serializedTree.substr(0, 1) == std::string("#"))
        return;
    int start = 0;
    root_ = deserializeTreeHelper(root_, serializedTree, &start);
}

Tree::Node*
Tree::deserializeTreeHelper(Node *root, std::string serializedTree, int *start) {
    if (*start == serializedTree.length()) return nullptr;
    if (serializedTree.substr(*start, 1) == "#") *start = *start + 2;
    if (*start >= serializedTree.length()) return nullptr;

    std::stringstream ss; ss << serializedTree.substr(*start, 1);
    int data; ss >> data;
    root = new Node(data);
    *start = *start + 1;
    root->left = deserializeTreeHelper(root->left, serializedTree, start);
    root->right = deserializeTreeHelper(root->right, serializedTree, start);
    return root;
}

I want to know if having this kind of operation-operationHelper method combination a good design or is there a better way to design the methods?

I think this is OK, but there are a few things I'd consider changing.

Your helpers should be protected or private, since they should only be being called by the non-helper methods.
You should consider the const-ness of your methods. e.g. you want void printTree() const.
If you're using this pattern a lot, you could consider using a visitor pattern for things that traverse the tree, and a builder style pattern for things that build the tree. This way printTree could be moved outside the class and implemented as something like

void printTree(const Tree& tree) {
  tree.visit( TreePrinter() );
}

And similarly the deserializeTree function would be replaced with a constructor like this

Tree tree( TreeBuilderFromString(rep) );

It looks like the code reads a balanced (or almost balanced) tree. But since presumably you can build a non-balanced tree, serialization followed by deserialization will not generate the original tree. (Maybe this is expected and OK, but it may surprise someone when they try to use the class)


Monte-Carlo method to check triangle inequality This is code for finding if inequality \$abc > 2(\text{area of triangle})\$
 where \$a\$, \$b\$ and \$c\$ are the sides of any triangle.

The below, and repl, code work as intended. However they take a long time. Can the performance be improved without reducing max_tries?

import random
import math
hn=100000000
num=0
max_tries=10000000
tries = 0
while tries != max_tries:
  a= random.randint(1,hn)
  b= random.randint(1,hn)
  c= random.randint(1,hn)
  if a+b>c and c+b>a and a+c>b :
    s=(a+b+c)/2
    tries= tries +1
    k =math.sqrt(s*(s-a)*(s-b)*(s-c))
    if a*b*c> 2*k:
      num= num+1

print(num/max_tries)

The problem of your code is the way you generate triangles. You just create three independent random numbers. In theory, you could always end up rolling numbers that don't end up in a valid triangle and thus your code would not even terminate. That is, because triangle side lengths are dependent.

To always create a valid triangle, you can instead roll two angles and one side and then use the law of sines to calculate the remaining sides.

The sum of angles in a triangle is 180 degrees (or pi radians). So the first angle alpha is randomly selected between (0, pi), the second angle beta is then between (0, pi-alpha) and the third angle gamma is gained by simple arithmetics: gamma = pi - alpha - beta

Or in code:

alpha = random.uniform(0, math.pi)
beta  = random.uniform(0, math.pi - alpha)
gamma = math.pi - alpha - beta

If we know two sides and the angle in between, we can easily calculate the area. Since your inequality uses twice the area, we can drop the factor of 0.5.

double_area = side_b * side_c * sin_alpha

Some micro-optimisations: math.sin is an expensive function (time-wise), so we save the result in a variable. Same goes for floating point division. Multiplication is faster, so once again, we save the result to use multiplication later on.

Lastly: Since you already know how often you want to execute the loop, you can use a for-loop instead of a while-loop although frankly, I don't know how well this goes in terms of memory consumption.

import math
import random
import time

max_side_length = 100000000
n_max_tries     = 10000000
n_successes     = 0

time_start      = time.time()

for k in range(n_max_tries):
    alpha                   = random.uniform(0, math.pi)
    beta                    = random.uniform(0, math.pi - alpha)
    gamma                   = math.pi - alpha - beta

    side_a                  = random.randint(1, max_side_length)
    sin_alpha               = math.sin(alpha)
    side_a_over_sin_alpha   = side_a / sin_alpha

    side_b                  = side_a_over_sin_alpha * math.sin(beta)
    side_c                  = side_a_over_sin_alpha * math.sin(gamma)

    double_area             = side_b * side_c * sin_alpha

    if side_a * side_b * side_c > double_area:
        n_successes += 1

time_end = time.time()

print("successes: ", n_successes, " / ", n_max_tries, " (", n_successes/n_max_tries, ")")
print("time: ", time_end-time_start)

I didn't do excessive timings because I couldn't be bothered, but this code runs on my machine in about 37.8s, while three runs of your code averaged on 109.8s, resulting in an optimistic speed-up factor of around 3.

Disclaimer: I can somewhat write in python but I'm probably violating this famous pep-8. Yet, I strongly encourage you to give reasonable names to your variables as it makes the code more easy to understand. Not only for others, but also for yourself, when you look at it 2 weeks later.


constexpr hash function Here's a constexpr hash function, that will pack a string into the largest unsigned integral type available. So, what do you think?
#include <climits>

#include <cstdint>

#include <utility>

#include <iostream>

namespace detail
{

template <typename T, std::size_t ...I>
constexpr T hash(char const* const s, std::size_t const N,
  std::index_sequence<I...>) noexcept
{
  return ((T(s[I < N ? I : 0]) << ((I < N ? I : 0) * CHAR_BIT)) | ...);
}

}

template <typename T = std::uintmax_t>
constexpr T hash(char const* const s, std::size_t const N) noexcept
{
  return detail::hash<T>(s, N, std::make_index_sequence<sizeof(T)>());
}

template <typename T = std::uintmax_t, std::size_t N>
constexpr T hash(char const(&s)[N]) noexcept
{
  return hash<T>(s, N - 1);
}

int main()
{
  std::cout << (hash("a") == 'a') << std::endl;

  return 0;
}

https://wandbox.org/permlink/KbPiWJc434xYLL3q

Your code is too complex. With C++17, you can write more complex constexpr functions, so you don't need the variadic template tricks:
template <typename T = std::uintmax_t, std::size_t N>
constexpr T hash(char const(&s)[N]) noexcept
{
  T val{};

  for (size_t i = 0; i < N; ++i)
    val |= s[i] << (i * CHAR_BIT);

  return val;
}

Apart from that, this is a terrible hash function! The output is highly correlated to the input. It will also only hash up to sizeof(T) characters, so long strings with a common prefix might all get the same hash value.


Handling page requests in MVC Is this an efficient way to handle page requests in MVC?

index.php:

require 'Classes/Autoloader.php';
Autoloader::start();
Session::start();

new Bootstrap();

Bootstrap.php

class Bootstrap {
    private $controller = null,
            $action = null,
            $args = null;

    public function __construct() {
        $this->manage_url();
        if($this->page_exist()){
            $this->request();
        } else {
            $error = new ErrorController();
            $error->error404();
        }
    }

    private function request(){
        $controller = new $this->controller();
        if(!$this->args){ $controller->{$this->action}(); }
        else{
           $count_args = count($this->args);
           switch ($count_args) {
               case 1:
                   $controller->{$this->action}($this->args[0]);
                   break;
               case 2:
                   $controller->{$this->action}($this->args[0], $this->args[1]);
                   break;
               case 3:
                   $controller->{$this->action}($this->args[0], $this->args[1], $this->args[2]);
                   break;
               case 4:
                   $controller->{$this->action}($this->args[0], $this->args[1], $this->args[2], $this->args[3]);
                   break;
           }
        }
    }

    /**
     * Get controller, action and parametes
     */
    private function manage_url(){
        $uri = $_SERVER['REQUEST_URI'];
        $uri = trim($uri, '/');
        $this->remove_query_or_hash($uri);
        $exploded_uri = explode('/', $uri);
        $this->controller = Util::istruthy_or($exploded_uri[0], 'Main').'Controller';
        $this->action = Util::istruthy_or($exploded_uri[1], 'index');
        $this->args = array_slice($exploded_uri, 2);
    }

    private function remove_query_or_hash(&$uri){
        $query = strpos($uri, '?');
        $hash = strpos($uri, '#');
        if($query!==FALSE||$hash!==FALSE){
            $idx =  $query < $hash ? $hash : $query;
            $uri = substr($uri, 0, $idx);
        }
    }

    private function page_exist(){
        $controller = class_exists($this->controller);
        $method = method_exists($this->controller, $this->action);
        if(!$controller||!$method){
            return FALSE;
        }
        return TRUE;
    }
}

Util istruthy_or():

public static function istruthy_or(&$var, $val = NULL){
    return isset($val) ? ($var ? $var : $val) : ($var ? $var : NULL);
}

MVC is a concept that stems from good OOP practices. It's about seperating your application to three different parts: InputControllers, ProcessingModel and OutputView.

What you are describing is the bootstrap page, which, at least from my perspective should not be a class.

Here's my approach to this:

index.php

<?php
require "../bootstrap.php";

bootstrap.php

<?php
//Pseudo-code ahead:
require autoloader;
start router;
add router rules (either from file, or actually in the code)
$route = $router->route($uri); //$route is a Route object which tells us
                               //what controller to use, what are the parameters, etc.
$controllerClass = $route->getControllerClass();
$controllerAction = $route->getControllerAction();
$controller = new $controllerClass($request);
//Where $request is the Request object, containing the URI, GET, POST, COOKIES, etc.
$viewParams = call_user_func_array([$controller, $controllerAction], $route->getParameters());
//$route->getParameters() is an array of parameters from the route.
//call_user_func_array will transform that array into actual arguments to pass in.

$viewClass = $viewParams["class"];
$viewAction = $viewParams["action"];
$view = new $viewClass($request);
echo $view->render($viewParams);


Generic Graph using Adjacency matrix - Java The main purpose of representing Graph using adjacency matrix method is, to check the vertex and its neighbor's existence in constant time proportional to \$\mathcal{O}(n)\$.

In the various tutorials I have seen, Graphs contain only integer vertices and it becomes straight forward to represent them in a \$v \times v\$ integer 2D array to map the vertices.

In real world, we might have to store a custom type Object as the graph vertex. For this, I have created a Graph implementation with Adjacency matrix. Can you please let me know of any feedback / improvements?

//V - type of Object stored on graph vertices
public class GraphAM<V> {

    //Maps vertex with its adjacency matrix index. O(1) to retrieve index of a vertex
    private Map<V, Integer> vertices;
    //To get vertex using index at O(1) time
    private List<V> verticesLookup;

    //adjacency matrix
    private int[][] adj;

    private int index;

    public GraphAM(int numVertices) {
        adj = new int[numVertices][numVertices];
        index = 0;
        vertices = new HashMap<>();
        verticesLookup = new ArrayList<>();
    }

    public void addEdge(V from, V to) {
        addVertex(from);
        addVertex(to);

        int fromIndex = vertices.get(from);
        int toIndex = vertices.get(to);
        adj[fromIndex][toIndex] = 1;
    }

    private void addVertex(V v) {
        if(!vertices.containsKey(v)) {
            vertices.put(v, index);
            verticesLookup.add(index, v);
            index++;
        }
    }

    public void bfs(V start) {
        Queue<V> queue = new LinkedList<>();
        boolean[] visited = new boolean[vertices.size()];

        queue.add(start);
        int index = vertices.get(start);
        visited[index] = true;

        while(!queue.isEmpty()) {
            V v = queue.poll();
            System.out.print(v + " ");

            List<V> adjacentVertices = getAdjacentVertices(v);
            for(V a : adjacentVertices) {
                int adjInd = vertices.get(a);
                if(!visited[adjInd]) {
                    queue.add(a);
                    visited[adjInd] = true;
                }
            }

        }

    }

    public void dfs(V start) {
        boolean[] visited = new boolean[vertices.size()];
        dfs(start, visited);
    }

    private void dfs(V v, boolean[] visited) {
        System.out.print(v + " ");
        int index = vertices.get(v);
        visited[index] = true;

        List<V> adjacentVertices = getAdjacentVertices(v);
        for(V a : adjacentVertices) {
            int aIndex = vertices.get(a);
            if(!visited[aIndex]) {
                dfs(a, visited);
            }
        }
    }

    private List<V> getAdjacentVertices(V v) {
        int index = vertices.get(v);
        List<V> result = new ArrayList<>();
        for(int i=0; i<adj[index].length; i++) {
            if(adj[index][i] == 1) {
                result.add(verticesLookup.get(i));
            }
        }
        return result;
    }

}

Main class

class Main {

  public static void main(String[] args) {
            GraphAM<Integer> g = new GraphAM<>(4);

            g.addEdge(0, 1);
            g.addEdge(0, 2);
            g.addEdge(1, 2);
            g.addEdge(2, 0);
            g.addEdge(2, 3);
            g.addEdge(3, 3);

            System.out.println("Following is Breadth First Traversal "+
                    "(starting from vertex 2)");

            g.bfs(2);

            System.out.println("\nFollowing is Depth First Traversal "+
                    "(starting from vertex 2)");

            g = new GraphAM<>(4);

            g.addEdge(0, 1);
            g.addEdge(0, 2);
            g.addEdge(1, 2);
            g.addEdge(2, 0);
            g.addEdge(2, 3);
            g.addEdge(3, 3);

            g.dfs(2);
    }

}

You can simplify the mapping you are using to get from vertex to it's associated object. Instead of using a List<V>, which can only guarantee \$\mathcal{O}(n)\$ lookup time, you could just use an array.
Comments restating what's known should be removed, comments that can be made obsolete through refactoring should prompt that refactoring:

public class GraphAM<V> {
    private Map<V, Integer> vertexToIndex;
    private V[] indexToVertex;

    private int[][] adjacencyMatrix;
    private int index;

vertices (aka vertexToIndex can be eagerly initialized.
all private fields (apart from index) should be marked final.
Instead of passing numVertices to the constructor, you should consider passing a Collection<V>, which removes the need to deal with adding vertices in addEdge.
Finally instead of using a boolean[] for visited, a Set<V> is significantly more obvious. It might come with a minor performance penalty though.
Finally the search functions do not search for anything... they're just performing an exhaustive traversal and are accordingly useless in their current form...


Traverse all switch branches without goto I'm wondering wether there is a way to walk through every branch of this switch without using goto

    public static List<KpiGroup> GetKpiGroups(int siteId, int topLevelProjectId, SiteDetailsModel.KpiCategory category, bool applyMapFilters)
    {
        var kpiGroups = new List<KpiGroup>();

        switch (category)
        {
            case SiteDetailsModel.KpiCategory.Any:
            case SiteDetailsModel.KpiCategory.Wbs:
                kpiGroups.Add(KPIGetActiveSite(siteId, topLevelProjectId));
                if (category == SiteDetailsModel.KpiCategory.Any)
                    goto task;
                break;

            case SiteDetailsModel.KpiCategory.Task:
        task:
                kpiGroups.Add(KPIGetTicketStatus(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetTicketSeverityClosed(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetTicketSeverityOpen(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetTicketOwnerOpen(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetTicketOwnerClosed(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetTicketActivity(siteId, topLevelProjectId));
                if (category == SiteDetailsModel.KpiCategory.Any)
                    goto sysConfig;
                break;

            case SiteDetailsModel.KpiCategory.SysConfig:
        sysConfig:
                kpiGroups.Add(KPIGetSysConfigOverview(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetSysConfigDeviceTypes(siteId, topLevelProjectId));
                kpiGroups.Add(KPIGetSysConfigLevel(siteId, topLevelProjectId));
                break;

            default:
                throw new ArgumentOutOfRangeException(String.Format("GetKpiGroups {0}", category.ToString()));
        }

        return kpiGroups;
    }

As Dan Lyons stated in his comment you could use the goto case ... instead of custom goto labels. Check the example on the link he provided.

If you want to avoid using goto completely. I suggest you take the adding of the items out of the switch/case and place that in methods. And then, in every case you call the appropriate method to add the necessary items. Example:

List<KpiGroup> kpiGroups = new List<KpiGroup>();

public static List<KpiGroup> GetKpiGroups(int siteId, int topLevelProjectId, SiteDetailsModel.KpiCategory category, bool applyMapFilters)
{
    switch (category)
    {
        case SiteDetailsModel.KpiCategory.Wbs: AddWbsGroups(); break;
        case SiteDetailsModel.KpiCategory.Task: AddTaskGroups(); break;
        case SiteDetailsModel.KpiCategory.SysConfig: AddSysConfigGroups(); break;
        case SiteDetailsModel.KpiCategory.Any:
            AddWbsGroups();
            AddTaskGroups();
            AddSysConfigGroups();
            break;
        default:
            throw new ArgumentOutOfRangeException(String.Format("GetKpiGroups {0}", category.ToString()));
    }

    return kpiGroups;
}

public void AddWbsGroups(int siteId, int topLevelProjectId)
{
    kpiGroups.Add(KPIGetActiveSite(siteId, topLevelProjectId));
}

public void AddTaskGroups(int siteId, int topLevelProjectId)
{
    kpiGroups.Add(KPIGetTicketStatus(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetTicketSeverityClosed(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetTicketSeverityOpen(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetTicketOwnerOpen(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetTicketOwnerClosed(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetTicketActivity(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetActiveSite(siteId, topLevelProjectId));
}

public void AddSysConfigGroups(int siteId, int topLevelProjectId)
{
    kpiGroups.Add(KPIGetSysConfigOverview(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetSysConfigDeviceTypes(siteId, topLevelProjectId));
    kpiGroups.Add(KPIGetSysConfigLevel(siteId, topLevelProjectId));
}

It looks like overkill using the methods but your code still looks clean, readable and maintainable. It's up to you, use this method or the suggestion from Dan Lyons.


Finding time complexity of my solution to the 'longest palindrome' problem in Python Here is a short version!

def longestPalindrome(string):

    # Will use this string to record the longest palindrome seen so far
    largestPalindrome = ""

    for index in range(0, len(string)):

        reqcharMatch = string[index]

        # Since a palindrome must have matching start and end characters
        # I will find the index of all occurrences of the current
        # character that I'm looking at in the remaining string.

        # Will accomplish this using regular expression or simply
        # by passing the character I'm looking to find, the remaining
        # string to another function, it will return a list having
        # indices where matching character is present.

        # Let us for example assume that, the character I'm at currently
        # is also present in indices 4, 8, 9.
        # otheroccurList = [4, 8, 9]

        otheroccurList = regexFunctionHere()

        for occurrenceIndex in range(len(otheroccurList)):

            originalString = string[index: otheroccurList[occurrenceIndex] + 1]

            reversedString = originalString[::-1]

            # If the string obtained by slicing from the current
            # character to the next occurence of the same character is
            # a palindrome, and the length of this particular string is
            # more than the largestPalindrome seen so far, then update
            # largestPalindrome to be equal to the current Palindrome
            # we are looking at.

            if((reversedString == originalString) and (len(originalString) >= largestPalindrome)):

                largestPalindrome = originalString

    # If literally no palindrome is found, then simply return the
    # first character in the string
    if largestPalindrome == "":
       return string[0]

    else:
       return largestPalindrome

Here is a longer version, please forgive me if its not refactored. I did my best to ensure good variable names, but you can copy paste this and run it with whatever test cases, it will pass them.

Long version:

class Solution:

    @staticmethod
    def findallOccurrence(findcharIndex, chartoFind, remString):

        """
        Function to find the all other indices where a character
        appears.
        """

        occurrenceList = []

        for i in range(len(remString)):

            if(remString[i] == chartoFind):

                index = findcharIndex + i + 1

                occurrenceList.append(index)
                index = 0

        return occurrenceList

    @staticmethod
    def reverseString(origString):
        """
        Function to reverse a string
        """

        return origString[::-1]

    @staticmethod
    def checkPalindrome(origString, reversedString):

        """
        Function to check if the original string and the reversed
        string is a palindrome.
        """

        if(origString == reversedString):
            return True

        else:
            return False

    def longestPalindrome(self, A):

        """
        This is my main function whose short version I've explained.
        """

        longestPalindrome = ""

        for index in range(len(A)):

            findChar = A[index]

            remainingString = A[index + 1:]

            oList = findallOccurrence(index, findChar, remainingString)

            for j in range(len(oList)):

                origString = A[index: oList[j] + 1]
                revString = reverseString(origString)

                isPalindrome = checkPalindrome(origString, revString)

                if(isPalindrome == True) and (len(origString) >= len(longestPalindrome)):

                    longestPalindrome = origString

        if(longestPalindrome == ""):
            return A[0]

        else:
            return longestPalindrome

sol1 = Solution()
sol1.longestPalindrome("abaaaaaa")

The reason I'm having trouble finding the time complexity of my solution is that there are many interlinked loops, and different function calls within them.. I think its O(n^2), but it looks like its O(n^3).

Some thoughts on the long version only:

Use at least one linter like pycodestyle on your code to learn to write more idiomatic Python. This makes the code easier to read for other Python developers.
Most of the names are good (longestPalindrome), but

index, i and findcharIndex in a single method is very confusing,
the semantics of A, oList, j and Solution all have to be inferred from context which is mentally taxing, and
type suffixes like in remString are generally discouraged.

if foo then return true else return false should be replaced with simply return foo in any language.
Ideally your algorithm should not need the if(longestPalindrome == ""): check at the end - longestPalindrome should contain the longest palindrome by then.
Many @staticmethods is a common anti-pattern indicating that the code is really imperative rather than object oriented. This code might work better as a @property of str, since semantically that is what it is.


React For loop component This is my React component for looping a component i.e. I don't want to use MAP or FOREACH in React Component's HTML.

import React from 'react';

/**
 * @param {items} from props
 * @description -> Works with array of items and render only one children object inside it, ith data object will be available in children component props.
 */

const RFor = (props) => {
    const { items } = props || [];
    return items.map((data, index) =>
{React.cloneElement(props.children, data)}
)
}

export default RFor;

And this is how I use to iterate my component.

<RFor items={TEMP_DATA_ARRAY}>
              <Item />
            </RFor>

What this will do is generate <Item /> till TEMP_DATA_ARRAY and each object is going to be available with data property inside <Item />.
I want to optimize it and this is the best approach I can come up with.

This approach is not bad but I'd highlight a few issues:

You wrap each element with a <div>, it's unnecessary and for a very long list you create many useless elements. This also prevents <RFor> to be conveniently used (for example) to generate rows in a table.
You're assigning data to the properties of each item. While this might be convenient it's also fragile: what if an object as a property named key, or children, ref or...anything else already used? I'd assign data to a single property.
const { items } = props || [] does not do what you, probably, think it's doing. [] won't be the default if items is falsy but the default if props is falsy. The correct syntax, if the value is undefined is const { items = [] } = props;. If you want to handle also null then you can't destructure: const items = props.items || [];.
To use index as key is, more often than not, a terrible idea if the list isn't fixed (for example if you can remove/add elements anywhere in the list). Leave this responsibility to the caller because it's the only one with enough knowledge to pick a sensible unique ID (which in some very specific cases can even be the index).
You're passing props.children directly to React.cloneElement however it expects a single element and children can be an array. Enforce this rule using React.only().

To put things together (leaving out the key for the moment):

const RFor = props => {
    const items = props.items || [];
    return items.map(data => React.cloneElement(React.only(props.children), { data });
};

All these said you may start wondering if you even need RFor, let's compare with few alternatives:

<RFor items={TEMP_DATA_ARRAY}>
    <Item />
<RFor>

Vs

{TEMP_DATA_ARRAY.map(data => <Item key={data.id} {...data} />)}

Or:

{TEMP_DATA_ARRAY.map(data => <Item key={data.id} data={data} />)}

I don't see any benefit (note that you may, instead of children use a property like:

<RFor items={TEMP_DATA_ARRAY} component={Item} />

Also note that to solve the key problem you might use a render function:

<RFor items={TEMP_DATA_ARRAY}>
    {data => <Item key={data.key} data={data} />}
<RFor>

RFor IMHO makes sense only if it adds any other value over map(), it might be filtering or it might be part of an abstraction to build a presentation component with templates (with properties like controlTemplate, headerTemplate and itemTemplate as you often see in WPF).


Aggregate sum by month I have JSON data:

var data = [ { dat: 'timestamp', val: 10 }, { dat: 'timestamp', val: 20}, (...)]

Data contains objects with timestamp and value, can be more than one in  month.

I need to sum values from objects which have the same month and year.

First I use each function to add additional field cat which contains formatted date in format MMM'YY for example: Sep'15, after that I group the result by this cat field. This result I map to objects with properties name and value. Values are summed by using reduce function on plucked values - val from each grouped result. I'm using underscore.

var withCategories = _.each(data, function (elem) {
                            elem.cat = moment(elem.dat).format("MMM[']YY");
                        });

var groupedResult = _.groupBy(withCategories , 'cat');

var finalResult = _.map(groupedResult, function (elems) {

                            var valuesArray = _.pluck(elems, 'val'); // from group
                            return {
                                name: elems[0] ? elems[0].cat : '',
                                value: _.reduce(valuesArray, function (memo, num) {
                                    return memo + num;
                                }, 0)
                            }
                        });

First of all, your indentation is odd. Use a beautifier tool like JSBeautifier. If you use Sublime Text 3, there's an HTML-CSS-JS Prettifier plugin which uses the same tool internally.

Next, dat and val don't really tell anything about your data. Name it meaningfully. It's a timestamp, maybe date? And value for val? If you worry about size on transmission, just use gzip. It's better than losing hair on misnamed data.

Next, you're using moment.js in one of the loops. Moment.js is very slow based on personal experience. It tries to do as much as possible to get dates correctly, sacrificing execution speed. If you profile your code, you'll see a lot of internal extend calls in moment to generate your date. Since you're just getting year and month, why not use native getFullYear and getMonth?

Now you look like you're just operating on arrays. You can use the native array methods instead of underscore (unless you're still trying to support IE8???)

So if I understand your code correctly, this should do the same thing with lesser loops.

// Because JS doesn't have a nice way to name months because they may differ per locale
var monthNames = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'];

// Use reduce to aggregate your data. Pass around a hash so that we have
// direct access to groups as well as ensure groups appear just once.
var dataByMonth= data.reduce(function(dataByMonth, datum){
  var date  = new Date(datum.dat);
  var value = datum.val;
  var month = monthNames[date.getMonth()];
  var year  = ('' + date.getFullYear()).slice(-2);
  var group = month + '\'' + year;

  dataByMonth[group] = (dataByMonth[group] || 0) + value;

  return dataByMonth;
}, {});

// Now just turn the hash into an array.
var finalResult = Object.keys(dataObjectByMonth).map(function(group){
  return { name: group, value: dataByMonth[group] };
});


Join strings from a List while also formating them The code I have wrote works fine, this inquiry being purely for educational purposes. I want to know how others would do this better and cleaner. I especially hate the way I add the list items to another list before they are joined. There has to be a more efficient way.

I realize an easy way to make this simple would be to store "OU=" and "DC=" in the database with their associated text, but that just feels unseemly to me.

I am building a string for the container argument of the PrincipalContext class for an LDAP call.

The "lst" List<string> contains DataRows of LDAP Organization Units like "Accounts", "Users", etc.

// Get ou list
List<string> lst = db.sda(sql).Rows.OfType<DataRow>().Select(dr => dr.Field<string>("txt")).ToList();

string OU = string.Empty;
List<string> lst = new List<string>();

foreach (string ou in Web.Info.Ldap.ouList)
{
    lst.Add(string.Format("OU={0}", ou));
}
OU = string.Join(",", lst);

Result:

  OU=Users,OU=Accounts,OU=Employees

I do the same thing to a list called dcList that produces the same kind of string:

DC = string.Join(",", lst);

Result:

  DC=severname,DC=another_value,DC=com

to which I join together with OU to get the complete string, like so:

string container = string.Join(",", OU, DC);

End result:

  OU=Users,OU=Accounts,OU=Employees,DC=sever,DC=othervalue,DC=com

String.Join can work with IEnumerable<T> so its not necessary to pass list to it. Same goes for foreach: you do not have to call ToList() at the end of your first query, you can loop through initial enumeration.
With LINQ you can join strings like that:

//you can call `ToList()` at the end, if you need to cache query results, but you dont have to
var fields = db.sda(sql).Rows.OfType<DataRow>().Select(dr => dr.Field<string>("txt"));
//you can merge this Select with the previous one, if you are not going to re-use `fields` enumeration
var ou = fields.Select(f => "OU=" + f);
var dc = someOtherFields.Select(f => "DC=" + f);

var result = String.Join(",", ou.Concat(dc));


Encapsulation, fields and properties for Asteroid class After changing all variables in this code from public to private and adding get/set according to the need to get modified and/or read in another class, I'd like to know if this was done correctly.

// Main
public class Asteroid
{
    Random random = new Random();

    private Rectangle boundingBox;
    public Rectangle BoundingBox
    {
        get { return boundingBox; }
    }

    private Vector2 position;
    public Vector2 Position
    {
        get { return position; }
    }

    private bool isVisible;
    public bool IsVisible { get; set; }

    private Vector2 origin;
    private float rotationAngle;
    private int speed;
    private Texture2D texture;
    private float randomX, randomY;

    // Constructor
    public Asteroid(Texture2D newTexture, Vector2 newPosition)
    {
        position = newPosition;
        texture = newTexture;
        speed = 4;
        randomX = random.Next(0, 750);
        randomY = random.Next(-600, -50);
        isVisible = true;
    }

    // Load Content
    public void LoadContent(ContentManager Content)
    {

    }

    // Draw
    public void Draw(SpriteBatch spriteBatch)
    {
        if (isVisible)
            // spriteBatch.Draw(texture, position, Color.White);
            spriteBatch.Draw(texture, position, null, Color.White, rotationAngle, origin, 1.0f, SpriteEffects.None, 0f);
    }

    // Update
    public void Update(GameTime gameTime)
    {
        // Set bounding box for collision
        boundingBox = new Rectangle((int)position.X - (texture.Width / 2), (int)position.Y - (texture.Height / 2), texture.Width, texture.Height);

        origin.X = texture.Width / 2;
        origin.Y = texture.Height / 2;

        // Update Movement
        position.Y = position.Y + speed;

        if (position.Y >= 950)
        {
            isVisible = false;
        }

        // Rotating Asteroid
        float elapsed = (float)gameTime.ElapsedGameTime.TotalSeconds;
        rotationAngle += elapsed;
        float circle = MathHelper.Pi * 2;
        rotationAngle = rotationAngle % circle;
    }
}

Fields vs Properties

As discussed in the comments above, there's a problem with IsVisible:

private bool isVisible;
public bool IsVisible { get; set; }

When this code compiles, you'll basically get something like this "under the hood":

private bool isVisible;

private bool IsVisible;
public bool get_IsVisible() { return this.IsVisible; }
public void set_IsVisible(bool value) { this.IsVisible = value; }

BUG!
You're using isVisible all over your code, but the setter for IsVisible, set_IsVisible, is never called. Hence your clients will not see the right value... literally: the public getter will always return default(bool)... false!

You really have two options.

Either you expose data, or you encapsulate data.

Think of what a class really is: it's a definition for a type. A type can be a class, but in C# you can also have an enum, a struct, an interface... types can also be abstract (like an interface, or an abstract class).

When you instantiate a class, you create an object of the type defined by the class. An object has an interface (do not confuse with interface) that exposes everything that's publicly available. The client code doesn't "see" anything that's private or protected (or internal if the client code is in another assembly). And that's exactly where encapsulation is: it's hidden, encapsulated inside the inner workings, the implementation details of the type - the interface doesn't expose it because client code does not need to know what the implementation details are.

Fields are implementation details. They don't need to be exposed, and doing so breaks encapsulation.

Option One

You could make IsVisible an auto-property:

public bool IsVisible { get; set; }

Or, for a get-only auto-property:

public bool IsVisible { get; private set; }

Remember that under the hood, there is a private field for that auto-property (see "When this code compiles..." above).

Option Two

You could make IsVisible a property that uses a _isVisible private backing field:

private bool _isVisible;

public bool IsVisible
{
    get { return _isVisible; }
    set { _isVisible = value; }
}

The get-only version simply omits the setter.

You'll notice I'm using a naming convention that makes it a no-brainer to tell a field (_isVisible) from a parameter (value - implicit parameter in a property setter).

If you don't like having an underscore prefix on your private fields (it really comes down to personal preference.. or to the naming conventions your team is using), you could alternatively do it like this:

private bool isVisible;

public bool IsVisible
{
    get { return this.isVisible; }
    set { this.isVisible = value; }
}

The this qualifier isn't really needed (it's actually redundant in this case), but then if you had a method like this:

private void DoSomething(bool isVisible)
{
    // "isVisible" in this scope refers to the parameter.
    // use "this.isVisible" to refer to the private field.
}

And this is why I prefer the underscore prefix.

Bottom line: Properties win.

Some useful references:

Member Design Guidelines (Field Design) on MSDN:

  We exclude constant and static read-only fields from this strict restriction, because such fields, almost by definition, are never required to change.

  X DO NOT provide instance fields that are public or protected. You should provide properties for accessing fields instead of making them public or protected.
  √ DO use constant fields for constants that will never change. The compiler burns the values of const fields directly into calling code. Therefore, const values can
  never be changed without the risk of breaking compatibility.
  √ DO use public static readonly fields for predefined object instances. If there are predefined instances of the type, declare them as public read-only static fields of the type itself.
  X DO NOT assign instances of mutable types to readonly fields.

Note that in oop, static, predefined instance is generally just bad wording for don't do that!!

Also while you certainly don't want a mutable readonly struc, but readonly in front of a reference type only means that the reference cannot be reassigned. These are guidelines after all ;)

Member Design Guidelines (Property Design) on MSDN:

  √ DO create get-only properties if the caller should not be able to change the value of the property. Keep in mind that if the type of the property is a mutable reference type, the property value can be changed even if the property is get-only.
  X DO NOT provide set-only properties or properties with the setter having broader accessibility than the getter. For example, do not use properties with a public setter and a protected getter. If the property getter cannot be provided, implement the functionality as a method instead. Consider starting the method name with Set and follow with what you would have named the property. For example, AppDomain has a method called SetCachePath instead of having a set-only property called CachePath.
  √ DO provide sensible default values for all properties, ensuring that the defaults do not result in a security hole or terribly inefficient code.
  √ DO allow properties to be set in any order even if this results in a temporary invalid state of the object. It is common for two or more properties to be interrelated to a point where some values of one property might be invalid given the values of other properties on the same object. In such cases, exceptions resulting from the invalid state should be postponed until the interrelated properties are actually used together by the object.
  √ DO preserve the previous value if a property setter throws an exception.
  X AVOID throwing exceptions from property getters.

Abstraction

If I look at the interface of an Asteroid, I see these members:

Asteroid(Texture2D newTexture, Vector2 newPosition);

Rectangle BoundingBox { get; }
Vector2 Position { get; }
bool IsVisible { get; set; }

void LoadContent(ContentManager content);
void Draw(SpriteBatch spriteBatch);
void Update(GameTime gameTime);

I would think pretty much everything that's drawable in your game has a very similar interface, if not an identical one.

If you defined an abstraction for the methods this interface exposes...

public interface IDrawableContent
{
    void LoadContent(ContentManager Content);
    void Draw(SpriteBatch spriteBatch);
    void Update(GameTime gameTime);
}

Then you could have every drawable game component implement that interface, like this:

public class Asteroid : IDrawableContent

And since your GameScreen (I'm making this up) also has these methods with the same signature, why not do this as well:

public class GameScreen : IDrawableContent

Your AsteroidZapperGame class (or whatever it's called) also has similar methods (defined by the XNA Game class), but it's pretty much your top-level object, so you don't need it to implement the interface.

What gives?

The GameScreen class might roughly look like this (over-simplified):

public class GameScreen : IDrawableContent
{
    // _items is readonly.
    // its reference may only be assigned in a constructor.
    // items may still be added or removed from the list any time.
    private readonly IList<IDrawableContent> _items;

    public GameScreen(IList<IDrawableContent> items)
    {
        _items = items;
    }

    public void LoadContent(ContentManager content)
    {
        foreach (var item in _items)
        {
            item.LoadContent(content);
        }
    }

    public void Draw(SpriteBatch spriteBatch)
    {
        foreach (var item in _items)
        {
            item.Draw(spriteBatch);
        }
    }

    public void Update(GameTime gameTime)
    {
        foreach (var item in _items)
        {
            item.Update(gameTime);
        }
    }
}

This class has no clue that you're making it draw asteroids. In fact, it could just as well be drawing bubbles in a soda can, or a bunch of zombies hunting the player down. That's what abstractions do: they abstract away the implementation details, and that makes code easier to read, extend, and maintain.


Legacy Data and Current System Data Merged together in a single SQL Database There was a lot going on in this Database that I couldn't change, a lot of bad practices because the data was originally being pulled from an old mainframe database and then transformed by a horrible looking VB service and inserted into several (horrible) tables in an SQL Database so the information could be used by a VB.NET website (1.1 or something like that?)

I would like to know how clean this code is and whether or not there is anything that can be done to further my knowledge in retrieving information from Legacy Data stores.

This was an Ad-Hoc, and from the looks of the first Where clause it would be a very interesting Stored Procedure to say the least.

WITH Table1 AS
(
    SELECT
        CaseParticipantTable.DOCK_NO AS CaseNbr, ActivityTable.JUDGMENT_DATE AS JudgmentDate, ActivityTable.AMOUNT_TYPE AS AmountType,
            ActivityTable.AMOUNT AS JudgmentAmount, CaseParticipantTable.FILING_NAME AS CaseParticipant
    FROM
        UJSJudgementQuery.dbo.CJ_SC_CASE_PARTICIPANT AS CaseParticipantTable
        INNER JOIN UJSJudgementQuery.dbo.CJ_SC_JUDGMENT_ACTIVITY AS ActivityTable ON CaseParticipantTable.DOCK_NO = ActivityTable.DOCK_NO
    WHERE CaseParticipantTable.FILING_NAME IN ('/CountyName COUNT AUDITORS OFFICE', '/CountyName COUNTY AUDITOR',
                    '/CountyName COUNTY AUDITOR''S OFFI', '/CountyName COUNTY AUDITORS OFFI', '/CountyName COUNTY AUDITORS OFFIC',
                    '/CountyName COUNTY AUDITORS OFFICE', '/CountyName COUNTY TREASURER', '/CountyName County Treasurer',
                    '/CountyName COUNTY/ AUDITOR', '/CountyName COUNTY/AUDITORS OFFIC', '/CountyName COUNTY C/O AUDITORS')
        AND AMOUNT > 0
), ActiveJudgments AS (
    SELECT Table1.*, Judgments.JUDGMENT_STATUS
    FROM
        Table1
        INNER JOIN UJSJudgementQuery.dbo.CJ_SC_JUDGMENTS AS Judgments ON Table1.CaseNbr = Judgments.DOCK_NO
    WHERE Judgments.JUDGMENT_STATUS = 'A'
), NoInactives AS (
    SELECT ActiveJudgments.*
    FROM ActiveJudgments
    WHERE ActiveJudgments.CaseNbr NOT IN (SELECT Table1.CaseNbr
                                            FROM
                                                Table1
                                                INNER JOIN UJSJudgementQuery.dbo.CJ_SC_JUDGMENTS AS Judgments ON Table1.CaseNbr = Judgments.DOCK_NO
                                            WHERE Judgments.JUDGMENT_STATUS = 'I')
)
SELECT DISTINCT
    NoInactives.CaseNbr
    , CaseParticipantTable.FILING_NAME AS Debtor
    , NoInactives.JudgmentDate
    , NoInactives.JudgmentAmount
    , NoInactives.JUDGMENT_STATUS
FROM
    NoInactives
    INNER JOIN UJSJudgementQuery.dbo.CJ_SC_CASE_PARTICIPANT AS CaseParticipantTable ON NoInactives.CaseNbr = CaseParticipantTable.DOCK_NO
WHERE (CaseParticipantTable.PARTICIPANT_CODE = 'DBT' OR CaseParticipantTable.PARTICIPANT_CODE = 'DEF')
ORDER BY Debtor ASC

The ORDER BY at the end is just for checking my data, it is either by date or by Debtor Name

I am looking for suggestions on future query writing

Obviously table1 is not a very good name, so I'll leave that as that. The other thing that immediately jumps at me is the poor formatting.

It's hard to read multiple items strung across a single line. Doubly when they're aliased.

SELECT
    CaseParticipantTable.DOCK_NO AS CaseNbr, ActivityTable.JUDGMENT_DATE AS JudgmentDate, ActivityTable.AMOUNT_TYPE AS AmountType,
        ActivityTable.AMOUNT AS JudgmentAmount, CaseParticipantTable.FILING_NAME AS CaseParticipant

Is easier to read like this:

SELECT
    CaseParticipantTable.DOCK_NO AS CaseNbr,
    ActivityTable.JUDGMENT_DATE AS JudgmentDate,
    ActivityTable.AMOUNT_TYPE AS AmountType,
    ActivityTable.AMOUNT AS JudgmentAmount,
    CaseParticipantTable.FILING_NAME AS CaseParticipant

Similarly, I don't like having the entire JOIN...ON statement on one line. I usually put the ON statement on the next line, indented one tab.

FROM
    UJSJudgementQuery.dbo.CJ_SC_CASE_PARTICIPANT AS CaseParticipantTable
    INNER JOIN UJSJudgementQuery.dbo.CJ_SC_JUDGMENT_ACTIVITY AS ActivityTable
        ON CaseParticipantTable.DOCK_NO = ActivityTable.DOCK_NO

I do like how you start an indented new line after keywords, but you don't do it consistently. This WHERE suffers from that. I also recommend a new line for the items in your in List.

WHERE
    CaseParticipantTable.FILING_NAME IN (
                                    '/CountyName COUNT AUDITORS OFFICE',
                                    '/CountyName COUNTY AUDITOR',
                                    '/CountyName COUNTY AUDITOR''S OFFI',
                                    '/CountyName COUNTY AUDITORS OFFI',
                                    '/CountyName COUNTY AUDITORS OFFIC',
                                    '/CountyName COUNTY AUDITORS OFFICE',
                                    '/CountyName COUNTY TREASURER',
                                    '/CountyName County Treasurer',
                                    '/CountyName COUNTY/ AUDITOR',
                                    '/CountyName COUNTY/AUDITORS OFFIC',
                                    '/CountyName COUNTY C/O AUDITORS')
    AND AMOUNT > 0

You may have your reasons for it, but there's a lot of duplication in your IN statement. Consider whether or not a LIKE...OR...IN type statement would meet your needs. It will perform worse, so test it against your original to see how bad the performance hit will be.

WHERE
    CaseParticipantTable.FILING_NAME LIKE '/CountyName%COUNT%AUDITOR%'
    OR CaseParticipantTable.FILING_NAME IN(
                                    '/CountyName COUNTY TREASURER',
                                    '/CountyName County Treasurer')

This is an adhoc query now, but many many adhoc queries make their way into production use. Never should you Select * in a production query. It often forces a table scan where a more efficient query might be found otherwise.


Return the element of collection if the collection contains only one element Recently I have found in my code several places where I was first gathering some solutions, and then continued processing them only if the solution was unique (solution collection contained only one element). Following code is an attempt to solve this in a more functional manner.

  implicit class GetOnlyOne[A](val coll: Iterable[A]) {
    def getonlyone = {
      if (coll.isEmpty) None
      else if (coll.tail.isEmpty) coll.headOption
      else None
    }
  }

The function can be used like:

Seq(1).getonlyone
Seq(1,2).getonlyone
Set(1).getonlyone

Is there anything missing to be idiomatic Scala, or to be more elegant?

I think that, considering the constraints of your situation (Iterable), the implementation is about as good as it's going to get.

However, that does not mean there aren't things that couldn't be improved.

Camel case is standard for method names. I find getOnlyOne to be easier to read than the all lower case version.
It has become more of a standard to declare the result type of methods, especially if the method is part of an API. Unless it's a one-line method whose return type is blindingly obvious.

implicit class GetOnlyOne[A](val coll: Iterable[A]) {
  def getOnlyOne: Option[A] = {
    if (coll.isEmpty) None
    else if (coll.tail.isEmpty) coll.headOption
    else None
  }
}


Reducing the sorting time of a large number of numbers I have to sort many large numbers in a given time limit. The program below works, but not when the test cases reaches \$10^6\$, in which case it exceeds the time limit.

def tsort(tests)
        sorted_array = []
        1.upto(tests) do
            input_num = gets.chomp.to_i
            sorted_array[input_num] = 1
        end
        #puts sorted_array.sort!
        sorted_array.each_index {|index| puts index if sorted_array[index] == 1}
    end

    test_cases = gets.chomp.to_i
    tsort(test_cases)

I wanted to ask if there was a better way to do this. Perhaps an algorithm will help me accomplish this.

Single Responsibility and Lying Names
The tsort method does too much. It both gets input from the user and stores that information in an array (a potentially large one at that, but more on that later). Then it prints the array. You should have three separate methods here and tsort should do one thing, sort an array. As it is, nothing actually gets sorted per se. You're just keeping the list sorted, which seems smart, but...
Performance
Think about what happens when I give gets.chomp.to_i the number 100. The number 1 gets placed into the 100th position of the array. To get it back out, we have to then iterate over 100 positions. If I give it 1000 instead, we now have to make 1000 iterations in order to find it. That's 10 times as many iterations. This method gets exponentially slower. It is no surprise that inputting 10^6 exceeds the time constraints; it takes 10^6 iterations to print the number, even if it's the only number in the array.
This can be solved by implementing any number of well studied sorting algorithms. A quick google search should get you started. I recommend learning to bubble sort first, then move on to the more efficient algorithms. This assumes you're doing this for educational purposes. Otherwise, just use Ruby's built in Array#sort method.


One object to handle all my database manipulation needs I have created an object to handle all my database manipulation needs. Here is my object constructor and one of its functions:

class dataMonster{
    var $sn = '';
    var $un = '';
    var $pw = '';
    var $dn = "";

    function update($table, $records, $set_value, $compare, $compare_value){

        $table=preg_replace('/[^0-9a-zA-Z_]/', '', $table);
        $records=preg_replace('/[^0-9a-zA-Z_,]/', '', $records);
        $compare=preg_replace('/[^0-9a-zA-Z_,]/', '', $compare);

        $dsn = "mysql:host=".$this->sn.";dbname=".$this->dn.";charset=utf8";
        $opt = array(
            PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
        );
        ini_set('display_errors', 'On');

        $return_check = $pdo = new PDO($dsn, $this->un, $this->pw, $opt);
        if($return_check === false){
            echo 'PDO failed';
        }
        $return_check = $stm = $pdo->prepare("UPDATE $table SET $records = ? WHERE $compare = ?");
        if($return_check === false){
            echo 'prepare failed';
        }
        $return_check = $stm->execute(array($set_value, $compare_value));
        if($return_check === false){
            echo 'execute failed';
        }
    }
}

It is then instantiated and called as such:

$dm = new dataMonster();
$dm->sn = 'localhost';
$dm->un = 'xtremeco_admin';
$dm->pw = 'fMMA1EJJuz8I';
$dm->dn = 'xtremeco_users';

$dm->update('users', 'sessionid', $session_id, 'username', $user_name);

The class constructor is held in a separate file and imported into the project I'm working on. I know the standard is to code in a whitelist of acceptable tables and columns to prevent injection but I think I have successfully safe guarded against it. No user input makes it into the statement itself. My goal is to create a file that can just be imported and run with no editing. But in doing so, am I leaving myself open to attack?

No user input makes it into the statement itself.

You should write a PHPDoc style comment which explicitely states that specific parameters should not contain user input.

This would of course also be the case if you used a whitelist to check against existing table and column names, as no user should have the ability to change arbitrary tables or columns.

  I have successfully safe guarded against it.

Pretty much. It's not clear to me why you accept , though. Without =, you can't add a second value anyways, so , should not be necessary.

Method Signature

  $dm->update('users', 'sessionid', $session_id, 'username', $user_name);

This isn't very easily understandable. Sure, it updates some table, but a reader has no idea what exactly is happening without looking at the update code itself.

If you go with something like a query builder, you might get code like this:

$dm->update('users')
    ->set('sessionid', $session_id)
    ->set([something else])
    ->where('username', $user_name)
    ->exec();

This would be a lot easier to read as it is clear what each argument actually does.

Functionality

Your method should be able to deal with setting multiple fields at once. For example by accepting an array of records and set_values.

Structure

Your update method shouldn't create a new pdo object on each call. Instead, reuse the same connection for all queries.

Your method should also not echo. It makes it very difficult to recover from errors, or to control what a user sees. Throw exceptions instead (pdo can actually do this for you with ERRMODE_EXCEPTION so there is no need to check here).

Your method should also not change ini settings. These should be set in some configuration class.

With this, your method may look like this:

function update($pdo, $table, $records, $set_value, $compare, $compare_value){

    $table=preg_replace('/[^0-9a-zA-Z_]/', '', $table);
    $records=preg_replace('/[^0-9a-zA-Z_,]/', '', $records);
    $compare=preg_replace('/[^0-9a-zA-Z_,]/', '', $compare);

    $stm = $pdo->prepare("UPDATE $table SET $records = ? WHERE $compare = ?");
    $stm->execute(array($set_value, $compare_value));
}

The PDO exceptions would be handled by your calling code. Depending on what is updated, you may want to log the error, inform the user, ignore the error, perform a different action, and so on.

Of course now the question is if this method is actually needed. Is this:

try {
    $dm->update($pdo, 'users', 'sessionid', $session_id, 'username', $user_name);
} catch ($pdoException) {
    // handle error
}

Really better than this:

try {
    $stm = $pdo->prepare("UPDATE users SET sessionid = ? WHERE username = ?");
    $stm->execute(array($session_id, $user_name));
} catch ($pdoException) {
    // handle error
}

? I don't really think so. The second is clearer, provides less room for error, isn't really longer, and is more flexible.

Misc

only display errors in development. It has a negative effect on security if they are enabled in production. This makes it especially important to only change the setting in some configuration file and not all over the place.
Your password is above average, but you should probably change it now :)
sn, un, pw, dn, opt are all bad names. If you write them out your code will be more readable.