Saves Scores of an Arithmetic Quiz This code is part of a larger code that generates an Arithmetic Quiz, although the code I have show below just saves the score that the user got on the test and their name. When saving the code it checks if the user has taken the quiz before. If the user has taken the quiz before it saves it as 'Older Score' or 'Oldest Score', however if the user has not taken the quiz before it generates a new row for them. Additionally it works out the average score and highest score for all users it has the data for. For my task I'm supposed to make the code as short as possible.

name = ""+first_name+" "+last_name
rows_data = []
class_file_names = ['Class 1.csv', 'Class 2.csv', 'Class 3.csv']
scan = open(class_file_names[int(math_class)-1], 'r+')
rows = csv.reader(scan)
for row in rows:
    rows_data.append(row)
scan.close()
class_file_names=['Class 1.csv', 'Class 2.csv', 'Class 3.csv']
csvfile=open(class_file_names[int(math_class)-1], 'w', newline="")
fieldnames = ['Student Name', 'Oldest Score', 'Older Score', 'Latest Score', 'Average', 'Highest']
writer = csv.DictWriter(csvfile, fieldnames=fieldnames)
writer.writeheader()
nameFound=False
for x in rows_data[1:]:
    if x[0] == name:
        x[3]=x[2]
        x[2]=x[1]
        x[1]=str(score)
        x[4]=(int(x[1])+int(x[2])+int(x[3]))/3
        x[5]=max(x[1],x[2],x[3])
        nameFound=True
    writer.writerow({'Student Name': x[0], 'Oldest Score': x[3], 'Older Score': x[2], 'Latest Score': x[1], 'Average': x[4], 'Highest' : x[5]})

if nameFound==False:
    writer.writerow({'Student Name': name, 'Latest Score': score, 'Older Score': '0', 'Oldest Score': '0', 'Average': score, 'Highest': score})
csvfile.close()
input("Test complete! Press anything to close.")

I'd recommend a few things:

PEP8
List comprehensions
str.format
with
Slices

But I'll show how they can make your code nicer.

Using str.format you can easily create name.
Alternately, if you don't want to use str.join:

name = '{} {}'.format(first_name, last_name)
name = ' '.join([first_name, last_name])

Using a list comprehension you can reduce the amount of code to create the rows_data.

rows_data = [row for row in csv.reader(scan)]

Using with you can remove the file.close calls.
This can lead to a very small amount of code to read the rows_data.

with open(class_file_names[int(math_class) - 1], 'r+') as scan:
    rows_data = [row for row in csv.reader(scan)]

I'll touch on PEP8, class_file_names should be all caps, so CLASS_FILE_NAMES as it's a constant.
You should also have a space either side of most operators, the largest exception are commas.

Finally slices makes your x code much easier to read.
To get the first three items in a list you can use list[:3] where for two you can use list[:2].
combining these together you can overwrite your existing scores with ease:

x[:3] = [str(score)] + x[:2]

Also your max code may be wrong as it may use the max from the string representation,
not from what the numbers mean.
And on the second test, you will average as if there has been three tests, where there has been two.

In all I'd use:

CLASS_FILE_NAMES = ['Class 1.csv', 'Class 2.csv', 'Class 3.csv']
FIELDNAMES = ['Student Name', 'Oldest Score', 'Older Score',
              'Latest Score', 'Average', 'Highest']

file_name = CLASS_FILE_NAMES[int(math_class) - 1]
name = '{} {}'.format(first_name, last_name)
with open(file_name, 'r+') as scan:
    rows_data = [row for row in csv.reader(scan)]

with open(file_name, 'w', newline="") as csvfile:
    writer = csv.DictWriter(csvfile, fieldnames=FIELDNAMES)
    writer.writeheader()
    name_found = False
    for x in rows_data[1:]:
        if x[0] == name:
            x[:3] = [str(score)] + x[:2]
            nums = [int(x) for x in x[:3]]
            x[4] = sum(nums) / 3
            x[5] = max(nums)
            nameFound = True
        writer.writerow({
            'Student Name': x[0],
            'Latest Score': x[1],
            'Older Score': x[2],
            'Oldest Score': x[3],
            'Average': x[4],
            'Highest': x[5]})

    if not name_found:
        writer.writerow({
            'Student Name': name,
            'Latest Score': score,
            'Older Score': '0',
            'Oldest Score': '0',
            'Average': score,
            'Highest': score})

input("Test complete! Press anything to close.")


Another Stack implementation in Java I'm trying to write my own stack implementation using LinkedList. It looks like it is doing what it should doing. But how does it look from a design perspective?

If I will go for job interview and show such things to a interviewer, will they say: "that's looks nice" or they rather will look at me with weird look asking "who let you in"?

import java.util.LinkedList;

public class Stack <T> {
private T t;
private LinkedList<T> stack;

public Stack() {
    this.stack = new LinkedList<>();
}

public void push(T t) {
    stack.add(t);
}

public boolean isEmpty(){
    if(stack.size()!=0){
        return false;
    } return true;
}

public T pop(){
    if(!isEmpty()) {
        return stack.removeLast();
    }
    return t;
}

public T peek(){
    if(!isEmpty()){
        return stack.getLast();
    }
    return t;
}

}

It looks like it is doing what it should be doing.

I don't understand that remark. In particular, you posted no unit tests or other evidence of the target code being exercised in usual cases and in edge cases.

    private T t;

What is that? The ctor doesn't affect it. It doesn't appear to contribute to any Stack invariants. Why can't pop() and peek() simply return null?

    if (stack.size() != 0) {
        return false;
    } return true;

Don't show that at an interview. Use the more natural return stack.size() == 0 instead.

For some workloads involving a large stack, this implementation will show worse performance than one that uses a doubly linked list.


Unit test for allowable checkers moves Is pattern matching within a unit test a code smell?
Do I really need to throw an exception just for my test to fail if the function under test receives invalid arguments (i.e. not Succeeded case value)?

Here's my test:

[<Test>]
let ``get available positions for black soldier`` () =

    // Setup
    let piece = Black ( BlackSoldier , (1,5) )

    // Test
    let available = startGame () |> getPositions
                                 |> optionsFor piece
    // Verify
    match available with
    | OptionsResponse.Succeeded coordinates ->
        let northWestAvailable = coordinates |> List.exists (fun pos -> pos = (0,4))
        let northEastAvailable = coordinates |> List.exists (fun pos -> pos = (2,4))

        (northWestAvailable && northEastAvailable) |> should equal true

    | _ -> failwith "Failed to get options"

Here's the function under test:

let optionsFor piece positions =

    let duplicatesFound = positions |> Seq.countBy id
                                    |> Seq.map snd
                                    |> Seq.exists (fun count -> count > 1)
    if duplicatesFound
    then OptionsResponse.DuplicatesNotAllowed positions
    else match piece |> isKing with
         | false -> let options = positions |> availablePositions
                                            |> List.filter (optionsForSoldier piece)
                    OptionsResponse.Succeeded options

         | true  -> let options = positions |> availablePositions
                                            |> List.filter (optionsForKing piece)
                    OptionsResponse.Succeeded options

Appendix:

type Coordinate = int * int

type Space =
    | Occupied  of Piece
    | Available of Coordinate

type OptionsResponse =
    | DuplicatesNotAllowed of Space list
    | Succeeded            of Coordinate list

Is pattern matching within a unit test a code smell?

I don't think so. When a tested method returns a discriminated union, then using pattern matching in the unit test is natural.

  Do I really need to throw an exception just for my test to fail if the function under test receives invalid arguments (i.e. not Succeeded case value)?

Yes, I think you should. The reason for that is that if the function incorrectly returns something other than Succeeded, your test needs to fail. And throwing an exception is how you do that.

Also, if you're a fan of terseness, you can write the List.exists predicate as e.g. ((=) (0,4)), instead of using fun:

let northWestAvailable = coordinates |> List.exists ((=) (0,4))

Or, even better, use should contain:

coordinates |> should contain (0,4)

This is shorter, clearer and will also lead to clearer test failures.


String puzzle - operations with numbers inside of strings Puzzle description: input is dimensions (length, width, heigth) of various presents.

Example:

2x3x1
5x9x10
1x40x2

Part 1: determine the area of paper needed to wrap the gifts, wich is equal to 2*(l*w + w*h + l*h) + the smallest between l*w, w*h and l*h for each gift.

Part 2: determine the length of ribbon needed to wrap the gift, wich is equal to l*w*h + smallest perimeter for each gift.

I learned about aoc recently and decided to start from 2015. Below is my code from the day 2 (problem link). How can I improve it? Is it too long?

import java.io.*;
import java.util.*;

public class adventofcode2 {
    public static void main(String[] args){
        System.out.println("input: ");
        Scanner input = new Scanner(System.in);
        String line;
        String text = "";
        while (!(line = input.nextLine()).isEmpty()){
            text += line + "\n";
        }
        calcTotal(text);
        System.out.println("Paper area used -  " + TotalPaper);
        System.out.println("Ribbon length used -  " + TotalRibbon);
    }
    /**
     * 1 -  Calculates the area of paper necessary to make the presents wrapping
     * 2 -  Calculates the length of ribbon necessary to the wrapping
     * @param presentsText list of strings in the style numberxnumberxnumber
     */
    private static void calcTotal(String presentsText){
        String[] presents = presentsText.split("\n");
        for (String present : presents){
            calcEachPresent(present.split("x"));
        }
    }
    /**
     * Iteration inside the calcTotal method
     * @param presentText array of each present string written in
     * the style numberxnumberxnumber
     */
    private static void calcEachPresent(String[] presentText){
        //Definition of the three dimensions of the present.
        int l = Integer.parseInt(presentText[0]), w = Integer.parseInt(presentText[1]), h = Integer.parseInt(presentText[2]);

        //Definition of the area of the sides in the present.
        int[] d = {l*w,w*h,h*l};

        int min = getMin(d);

        int area = 2*(d[0] + d[1] + d[2]) + min;

        int ribbon = (l*w*h) + smallestPerimeter(l,w,h);
        TotalPaper += area;
        TotalRibbon += ribbon;
    }
    /**Get the smallest area to use in the area calculation
     *
     * @param num array of numbers
     * @return smallest number in an array
     */
    private static int getMin(int[] num){
        int min = num[0];
        for (int n : num){
            if(n < min) min = n;
        }
        return min;
    }
    /**Get smallest perimeter to use in ribbon length calculation
     *
     * @param l length of present
     * @param w width of present
     * @param h heigth of present
     * @return smallest perimeter
     */
    private static int smallestPerimeter(int l,int w,int h){

        int[] num = {l,w,h};
        Arrays.sort(num);

        return 2*(num[0] + num[1]);
    }

    /**
    *   VARIABLES TO RETURN IN THE MAIN METHOD
    */
    private static int TotalPaper = 0;
    private static int TotalRibbon = 0;
}

The structure of your code looks quite good so far but you could improve readability:

Naming Conventions

As with almost every other programming language java has a schema of how to name identifiers that's common consensus You can read the details here:
 http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-135099.html

The main line is: classes start with an uppercase letter, method and variable names start with lower case letter. And if the name consists of multiple words use CamelCase.

variable declarations

You declared 2 variables ab the end of your class file. The  common consensus in Java is to write class (or object) scope variables at the top of the class file.

less comments ##

there should be 2 types of comments in your code:

Description of the contract of an interface method
this type  of comment should guide the implementer of that method and tell what the caller of the method expects, especially what the properties of the parameters and the result are like.
Explanations why the code is like it is
this should tell the reader why you chose a certain approach.

There should not be any other comment in your code.

Whenever you feel the need to write a comment try to find better identifier names first.

Especially comments like this are not needed instead they clutter the code:

 /**Get smallest perimeter to use in ribbon length calculation
     *
     * @param l length of present
     * @param w width of present
     * @param h heigth of present
     * @return smallest perimeter
     */
    private static int smallestPerimeter(int l,int w,int h){

Instead the code could be like this:

private static int getSmallestPerimeterToUseInRibbonLengthCalculation(int length, int width, int heigth){

You may argue that the name is to long and you're right: the method should only tell what it does and not restrict the possible callers, so it should better be:

private static int getSmallestPerimeter(int length, int width, int heigth){

And get is not correct since this method is not a getter in the sense that the content of an object (or class) variable is returned, it calculates the returned value and the name should reflect that:

private static int calculateSmallestPerimeter(int length, int width, int heigth){

You use comments to structure a method:

  //Definition of the three dimensions of the present.
        int l = Integer.parseInt(presentText[0]), w = Integer.parseInt(presentText[1]), h = Integer.parseInt(presentText[2]);

  //Definition of the area of the sides in the present.

In such cases you should better extract that code between the comments into separate methods or objects with names derived from the comments:

PresentDimensions.java

  class PresentDimensions{
    final int length, height, width;
    PresentDimensions(int length, int width, int height){
       this.length  =  length;
       this.width  =  width;
       this.height  =  height;
    }
    public int[] calculateSurfaceAreas(){
       return new int[]{length*width,width*height,height*length}
    }

in calcEachPresent

 PresentDimensions presentDimensions = new PresentDimensions(presentText[0], presentText[1], presentText[2]);
 int[] d = presentDimensions.calculateSurfaceAreas();


Insert into array in reverse while iterating forward So currently I have this function:
function loadPath(chunkSize, dataSize, depth, index) {
  let path = []
  let i = 0
  while (i < depth) {
    path.unshift(index % chunkSize)
    index = Math.floor(index / chunkSize)
    i++
  }
  return path
}

I would now like to pass in path as a variable from an object pool of short path arrays. So for example:
function loadPath(path, chunkSize, dataSize, depth, index) {

}

let path = new Array(3)
loadPath(path, 5, 100, 3, 41)
console.log(path) // 1, 3, 1
loadPath(path, 5, 100, 3, 50)
console.log(path) // ...

The question is, how can I do that without using push/unshift or reverse operations on the array? Basically just treating the array as a static block of memory, while at the same time not adding any unnecessary operations.
function loadPath(path, chunkSize, dataSize, depth, index) {
  let i = 0
  let x = depth - 1
  while (i < depth) {
    path[x] = index % chunkSize
    index = Math.floor(index / chunkSize)
    i++
    x--
  }
}

My question is, can this be done with any fewer temp variables? Some magic along the lines of this?
function loadPath(path, chunkSize, dataSize, depth, index) {
  let i = 0
  while (i < depth) {
    // something tricky perhaps here?
    // since x and i have a reciprocal relationship...
    // somehow fill it in backwards...
    path[depth - i] = index % chunkSize
    index = Math.floor(index / chunkSize)
    i++
  }
}

Or perhaps there is another way entirely to write this algorithm to be in as few steps and operations and temp variables as possible.
Could also use this floor implementation integrated somehow if it helps :)
function floor(n) {
  return ~~n - (~~n > n)
}

Let's try the other way:

function loadPath(path, chunkSize, dataSize, depth, index) {
  for(var i = 0; i < depth; i++) {
    path[depth - i - 1] = index % chunkSize;
    index = Math.floor(index / chunkSize);
  };
};

var path = Array(3);
console.log("loadPath(path, 5, 100, 3, 41)");
loadPath(path, 5, 100, 3, 41);
console.log(path);
console.log("loadPath(path, 5, 100, 3, 50)");
loadPath(path, 5, 100, 3, 50);
console.log(path);
.as-console-wrapper { max-height: 100% !important; top: 0; }

What do you want to do if index >= chunkSize^3?


Parsing users from a list of commands for email notifications I want to make a list of the last words on each line of a file that looks like this:

casper@casper-PC ~/scripts -> cat /tmp/foo
update PTMM_ARCHIVE.FASTTRACK_USER set user_name = 'monohajoxx' where user_name = 'monohajo'
update PTMM_ARCHIVE.FASTTRACK_USER set user_name = 'wuemxx' where user_name = 'wuem'
update PTMM_ARCHIVE.FASTTRACK_USER set user_name = 'taraziemxx' where user_name = 'taraziem'
update PTMM_ARCHIVE.FASTTRACK_USER set user_name = 'mullankexx' where user_name = 'mullanke'
update PTMM_ARCHIVE.FASTTRACK_USER set user_name = 'fernanjaxx' where user_name = 'fernanja'casper@casper-PC ~/scripts ->

As you can see, the file lacks a newline at the end: 'fernanja'casper@casper-PC ~/scripts ->. I hate that when the prompt gets mixed in with the data -or the output. It's a minor point, but it makes the text on the terminal jumbled and confusing.

Also, I use tr all the time to fix my one-liners. I am looking for another way to print out the names in a horizontal line instead of a vertical list.
I echo'd a new line but it printed it out literally.

09:38 PM Tue Dec 01$ awk '{print $NF}' /tmp/foo | tr -d "'" | tr "\n" ", \s" ; echo  "\n"
monohajo,wuem,taraziem,mullanke,fernanja,\n
walt@walt-PC ~/scripts ->
09:55 PM Tue Dec 01$

I do not want this:

monohajo
wuem
taraziem
mullanke
fernanja

I want this:

monohajo, wuem, taraziem, mullanke, fernanja

These are minor points, but when you do this 20 or thirty times a day, it gets tiring. It would be great just to do this in pure bash, with a while loop. Instead of vertical lists, I need horizontal lists which can be cut and pasted into emails, so I can tell bosses that I performed updates on this list of people.

Critique

First, note that you are misusing tr when you do tr "\n" ", \s".  In tr string1 string2, string1 is supposed to be at least as long as string2, as each character in string1 is translated into the corresponding character in string2.  (If string1 is longer than string2, the last character found in string2 is duplicated until string1 is exhausted.)  Therefore, tr "\n" ", \s" is equivalent to tr "\n" ",", which produces a comma-separated output with no spaces — in contradiction to your sample output.

The dangling ; echo "\n" at the end is awkward.  You're now running two pipelines whose outputs happen to appear one after the other, rather than producing one output stream.  That means that the output merely looks right on screen.  You can't pipe or redirect the whole thing, though, unless you run the two pipelines in a subshell, like ( awk … | tr -d … | tr … ; echo "\n" ).

Suggested solution

awk '{ print $NF }' /tmp/foo | xargs echo will get you mostly there.  By using xargs, you automatically get unquoting behaviour.  (The way xargs handles quoting is not quite the same as the way SQL handles quoting, but in most cases, it's probably close enough.)  The echo command will produce one line of output, ending with a newline.

The only thing remaining is to insert a comma before each space.  For that, I would use | sed -e 's/ /, /g'.  (If you just want commas and no spaces, you can use tr ' ' , instead.)

In summary, do

awk '{ print $NF }' /tmp/foo | xargs echo | sed -e 's/ /, /g'


Soldier class, with health points Recently I completed the encapsulation lesson and I made a simple android app implementing encapsulation, I had difficulties understanding encapsulation.

My Soldier class:

public class Soldier {
    //Start of the class.

    private int solHealth;

   /* Soldier class constructor with the code to set the soldier health to 100
   * when ever we create a soldier object.*/

    public Soldier(){

        this.setSolHealth(100);
    }

    //setter method with the parameter to set the solHealth variable.

    private void setSolHealth(int solHealth) {
        this.solHealth = solHealth;
    }

    //getter method to get the solHealth value.

    public int getSolHealth(){

        return this.solHealth;
    }

    //Main method to heal the soldier by 25 every time this method is called.

    public int heal(){

        this.solHealth += 25;

        //Log used for debugging.

        Log.i("Info", "" + solHealth);

        return solHealth;
    }

    //Main method to injure the soldier by 25 every time this method is called.

    public int injure(){

        this.solHealth -= 25;

        //Log used for debugging.

        Log.i("Info", "" + solHealth);

        return solHealth;
    }

    //End of the class.
}

I created a soldier object in the MainActivity class and used it to access getSolHealth(), heal(), and injure() methods, and to manipulate solHealth values when the button is pressed.

Did I succeed implementing encapsulation correctly ?

The short answer is yes, you succeeded in implementing encapsulation 100% correctly.

Still, I have some smaller problems with your code:

Useless method

I noticed that you have a setter method for the Variable solHealth, which you should rename in my opinion to health. The problem is that the setter is useless as the method has the private access modifier and this method should never be used by the class itself. The only thing that you need to change when deleting this method is that you would have to change the constructor to be like this:

public Soldier() {
    solHealth = 100;
}

This would still follow the concept of encapsulation, as encapsulation is simply a process of wrapping code and data together into a single unit, that allows only minimal manipulation from outer classes.

Comments

If you are commenting your code you can easily implement documentation comments with which you can create JavaDoc pages. This is the standard of how to document methods, classes, and global variables/constants.

Example

/**
 * Set's the soldiers health to the specified value
 *
 * @param health
 */
public void setHealth(int health) {
    // Code here
}


Generating HTML code dynamically with jQuery I am doing a college project and I would like to write my jQuery code following best practices. The project involves using the jQuery .getJSON method to retrieve information from a json file. I have left out parts of the code so that it is more readable (however this is indicated in the comments).

The code shows two approaches I have come up with:

In the first approach, I define a variable which contains HTML code and inserts this dynamically when making a call to the JSON file. I then do a small amount of DOM manipulation with this.
In the second approach I don't need to define the variable at all. However, there is slightly more jQuery DOM manipulation when retrieving the information from the json file. also I'm not sure if what I have done is a bit 'hacky'.

Do you have any advice on which way would be better practice to do?

Option 1:

$(document).ready(function(){

//define a variable which contains html code that will be inserted dynamically depending on the contents of the json
        var gift_love_template =    '' +
                            '' +
                            '' +
                        '';

        //the following code is inside the call to the JSON file:
        $.each(profile.posts, function(i, post){
            var quantity_love = post.gifts.love;    //number of loves is stored in the gifts array within the post object.

            if(quantity_love != 0){
                var love_title = "" + quantity_love + " loves received"; //create a string for the title attribute
                $("#" + post_id + " .gift-area").append(gift_love_template);
                $("#" + post_id + " .love-gift").attr("title", love_title).append("(" + quantity_love + ")");
            }

        });
});

Option 2:

$(document).ready(function(){

    //the following code is inside the call to the JSON file:

        $.each(profile.posts, function(i, post){
            var quantity_love = post.gifts.love;    //number of loves is stored in the gifts array within the post object.

            if(quantity_love != 0){
                var love_title = "" + quantity_love + " loves received"; //create a string for the title attribute
                $('(' + quantity_love + ')').attr("class", "gift").attr("title", love_title).appendTo("#" + post_id + " .gift-area");
            }

        });
});

I suggest you put the template html in an invisible div in the page html.
Then reference and clone it from javascript.
Then replace "tags" with data from the json request and insert the new html in the page at the appropriate place.

You can then redesign the page without touching the javascript code.


How to Make This JavaScript Snippet DRY (Don't Repeat Yourself)? I have the following JavaScript function:

function registerEvents() {
    var isTapholding = false;
    $(document).delegate('.add-control .add-image', 'tap', function (evt) {
        handleAddControlEvent(this);
    });
    $(document).delegate('.delete-image', 'tap', function (evt) {
        evt.stopPropagation();
        handleDeleteControlEvent(this);
    });
    $(document).delegate('.edit-control', 'tap', function (evt) {
        if (isTapholding)
            return;
        handleEditControlEvent(this);
    });
    $(document).delegate('.edit-control', 'vmouseup', function (evt) {
        setTimeout(function () {
            isTapholding = false;
        }, 100);
    });
    $(document).delegate('.add-control .insert-image', 'tap', function (evt) {
        handleInsertImageEvent(this);
    });
    setTimeout(function () {
        $(document).delegate('.edit-control', 'taphold', function (evt) {
            evt.stopPropagation();
            isTapholding = true;
            handleEditControlTapHoldEvent(this);
        });
    }, 100);
}

How I can make this DRY (Don't Repeat Yourself)?

You can refactor this to follow DRY by using the Strategy Pattern.

function registerEvents(){
    var delegationStrategies = [
        {var1: '.add-control .add-image',
         var2: 'tap',
         func: function (evt) {
             handleAddControlEvent(this);
         }
        },
        {var1: '.delete-image',
         var2: 'tap',
         func: function (evt) {
             evt.stopPropagation();
             handleDeleteControlEvent(this);
         }
        },
        // Add more strategies here.
    ];

    for (var i=0; i< delegationStrategies.length; i++){
        strategy = delegationStrategies[i];
        $(document).delegate(strategy.var1, strategy.var2, strategy.func);
    }
}

You may need to add a special case for the timeout.

The question is whether you foresee the need to add further delegation handlers in the future.  If so, then it is a matter of adding another entry to the strategy.


Is the singly linked list a palindrome? I have solved this question on Leetcode: https://leetcode.com/problems/palindrome-linked-list/description/. I convert the nodes into an ArrayList and am wondering if that's cheating? Other than that, how can I optimize this solution:

public class PalindromeLinkedList {

    public static void main(String[] args) {
        ListNode node = new ListNode(0);
        node.next = new ListNode(0);
        //node.next.next = new ListNode(1);
        //node.next.next.next = new ListNode(1);

        ArrayList firstHalf = new ArrayList();
        ArrayList secondHalf = new ArrayList();

        int count = 0;

        ListNode counterNode = node;
        while(counterNode != null) {
            count++;
            counterNode = counterNode.next;
        }

        int middle = count/2;
        boolean middleIgnored = count%2 != 0;
        for(int i = 0; i < middle; i++) {
            firstHalf.add(node.val);
            node = node.next;
        }

        if(middleIgnored) {
            node = node.next;
            System.out.println("middle ignored");
        }

        for(int i = middle + 1; node != null; i++) {
            secondHalf.add(node.val);
            node = node.next;
        }

        Collections.reverse(secondHalf);
        boolean isPalindrome = firstHalf.equals(secondHalf);

        System.out.println("first half: " + firstHalf);
        System.out.println("\nsecond half: " + secondHalf);
        System.out.println("\npalindrome? " + isPalindrome);

    }

}

class ListNode {
    int val;
    ListNode next;
    ListNode(int x) { val = x; }
}

LeetCode seems to stress preparation for technical interviews, I consider it prudent to start sketching the most simple thing that could possibly do (I take the liberty to substitute the original last word work).
Things I like about your approach:

conceptually simple & clean:
comparing first and last half with one reversed

relying on runtime support in the environment of choice
for comparison and reversal

I do not consider transforming data to facilitate processing cheating, unless it explicitly violates part of the task description.
Things I don't like (this far, you saw it coming) about the execution (of said approach):

missing doc comments

e.g. about a known shortcoming:
using O(n) additional space out of convenience, not need

not defining an explicit predicate isPalindrome()
transforming more than one part of the linked list separately

using repeated code (this may be appropriate in an interview setting, if commented (preferably in the code sketch, too))

declaring the Lists for what they are (implementing class) instead of what use they are (interface)

Giving it a try:

/** Checks a home-grown linked list
 * for palindrome using linear additional space.
 * @param node the first node of the list to check, or null
 * @return the order of values in the list
 *  starting from node stays the same when reversed
 */// compare first half to reverse of second half
static boolean isPalindrome(final ListNode node) {
    final List<Integer> items = new java.util.ArrayList<>();

    for (ListNode n = node ; n != null; n = n.next)
        items.add(n.val);

    int count = items.size(),
        middle = count/2; // middle item in neither sublist for odd count
    final List<Integer> tail = items.subList(count-middle, count);
    java.util.Collections.reverse(tail);
    return count < 2 ||
        items.subList(0, middle).equals(tail);
}
/** Builds a node list from values & prints result of isPalindrome() */
static void checkNodeList(String values) {
    ListNode node, n = node = new ListNode(42); // dummy
    for (byte c: values.getBytes())
        n = n.next = new ListNode(c);
    node = node.next; // skip dummy
    System.out.println("palindrome(" + values
              + "): " + isPalindrome(node));
}
public static void main(String[] args) {
    checkNodeList("");
    checkNodeList("?");
    checkNodeList("!!");
    checkNodeList("codedoc");
    checkNodeList("codedoC");
    checkNodeList("cOdedoc");
    checkNodeList("Maddam");
    checkNodeList("maddam");
    checkNodeList("madDam");
}

[optimizing] this solution - I can't make approach or code any clearer.
LeetCode's follow on: can a single solution achieve O(n) time and O(1) space?:
I know how to do it modifying the input, I doubt it can be done without.
There are many ways to skin cats, thinking how easy it would be using python, I gave turning ListNodes into Lists a try, with loop jamming and use of a sentinel thrown in:

/** Checks a home-grown linked List
 * for palindrome using constant additional space.
 *
Temporarily modifies the list starting with node.
 * @param node null, or the first node of the
 *  List to check
 * @return the order of values in the List
 *  starting from node stays the same when reversed
 */
static boolean isPalindrome(final ListNode node) {
    if (null == node)
        return true;
    ListNode
        last,   // traverses list two steps/turn, stops at last node
        middle, // middle of list part already traversed by last
        reversed = null, // reverse of first half
        slow = middle = last = node;
    // jam "counting" and reversal of first half
    for (ListNode next ; ; last = next.next) {
        next = last.next;
        middle = slow.next;
        if (null == next)
            break;
        slow.next = reversed;
        reversed = slow;
        slow = middle;
        if (null == next.next) {
            last = next;
            break;
        }
    }
    if (null == reversed)
        return true;
    boolean mayBePalindrome = last.val == node.val;
    if (mayBePalindrome) {
        last.val = ~node.val; // turn last into sentinel
        // jam "check" and restoration of first half; single comparison
        while (reversed.val == middle.val) {
            ListNode n = reversed.next;
            reversed.next = slow;
            slow = reversed;
            reversed = n;
            middle = middle.next;
        }
        mayBePalindrome &= middle == last;
        last.val = node.val; // restore
    }
    while (null != reversed) {
        ListNode n = reversed.next;
        reversed.next = slow;
        slow = reversed;
        reversed = n;
    }
    return isPalindrome;
}

/** Prints result of isPalindrome() */
static void check(String init) {
    ListNode node = null == init
        ? null : new ListNode(init, null);
    System.out.println("palindrome(" + node
              + "): " + isPalindrome(node));
}

public static void main(String[] args) {
    check(null);
    check("'");
    check("cc");
    check("codedoc");
    check("codedoC");
    check("cOdedoc");
    check("Maddam");
    check("maddam");
    check("madDam");
}

/** Singly-linked list node class bolstered some to support
 * java.util.List iteration. */
static class ListNode extends java.util.AbstractList<Integer>
    implements List<Integer>
{
    int val;
    ListNode next;
    ListNode(int x) { val = x; }
    ListNode(int x, ListNode n) { val = x; next = n; }
    ListNode(String init, ListNode n) {
        next = n;
        if (null != init && 0 < init.length()) {
            val = init.charAt(0);
            if (1 < init.length())
                next = new ListNode(init.substring(1), n);
        }
    }
    @Override
    public Integer get(int index) {
        if (0 == index)
            return val;
        if (index < 0)
            throw new IllegalArgumentException("index < 0");
        if (null == next)
            throw new IllegalArgumentException("size <= index");
        return next.get(index-1);
    }
    @Override
    public int size() { return null == next ? 1 : 1 + next.size(); }
    @Override
    public Iterator<Integer> iterator() {
        return new Iterator<Integer>() {
                ListNode head = ListNode.this;
                @Override
                public boolean hasNext() { return null != head; }
                @Override
                public Integer next() {
                    int v = head.val;
                    head = head.next;
                    return v;
                }
            };
    }
}


Execute multiple query from a text file I am just wondering if this is a correct way to execute multiple query from a text file.

Connection conn = Utility.getConnection();
Statement stmt = null;
try{
    JFileChooser chooser = new JFileChooser();
    int returnVal = chooser.showOpenDialog(chooser);
    if(returnVal == JFileChooser.APPROVE_OPTION) {
       String fileName=chooser.getCurrentDirectory().toString()+File.separator+chooser.getSelectedFile().getName();
       FileInputStream fstream = new FileInputStream(fileName);
       DataInputStream in = new DataInputStream(fstream);
       BufferedReader br = new BufferedReader(new InputStreamReader(in));
       String strLine;
       if(conn != null)
           stmt = conn.createStatement();
       while ((strLine = br.readLine()) != null)   {
           if(stmt !=null){
               int rc=stmt.executeUpdate(strLine);
               if(rc == 0)
                   System.out.println("executing:  "+strLine);
           }
       }
       stmt.close();
       conn.close();
       in.close();
    }
}catch (Exception e){
    e.printStackTrace();
}

Some ideas:

Separate the GUI code from the logic.
If you stay with your current mixed code fail fast. If conn is null there no sense to open the file or ask the user to choose one.
JFileChooser.getSelectedFile() returns a File instance, pass it to FileInputStream directly, don't concatenate strings. File contains the absolute path.
It's a good practice to pass a Charset (or "UTF-8") the the constructor of the InputStreamReader. The default could vary from system to system.
The DataInputStream looks unnecessary. You can pass the FileInputStream to the InputStreamReader directly.
Consider using Guava's readLines.
Maybe you want to report all executed queries, so the if (rc == 0) condition is unnecessary.

Plus it lacks of proper exception/error handling and resource closing as others mentioned in the comments.


A Java class for reading MaCH dosage files v2.0 Version 2 of A Java class for reading MaCH dosage files

import java.io.*;
import java.util.ArrayList;
import java.util.regex.Pattern;
import java.util.zip.GZIPInputStream;

/**
 * Created by vu.co.kaiyin.ReadMachDosage on 15/11/14.
 */
public class Dosage {
    final String filename;
    final Pattern splitPattern;

    public Dosage(String filename) throws Exception {
        this.filename = filename;
        splitPattern = Pattern.compile("\\s+");
    }

    public double[][] readCols(int start, int end) throws IOException {
        try(
                InputStream fileStream = new FileInputStream(filename);
                InputStream gzipStream = new GZIPInputStream(fileStream);
                Reader reader = new InputStreamReader(gzipStream, "US-ASCII");
                BufferedReader bufferedReader = new BufferedReader(reader);
                ) {
            String line;
            String[] fields;
            ArrayList<double[]> dosageList = new ArrayList<>();
            boolean firstLine = true;
            int nCol;
            while ((line = bufferedReader.readLine()) != null) {
                fields = splitPattern.split(line);
                if(firstLine) {
                    nCol = fields.length - 2;
//          Data indices: x      x    1      2      3
//               Indices: 0      1    2      3      4
//               Columns: 1->241 DOSE 1.1783 0.3498 1.9834 ...
                    if(start < 1) start = 1;
                    if(end > nCol) end = nCol;
                    start++;
                    end += 2; // java end index is exclusive
                    firstLine = false;
                }
                double[] dosage = new double[end - start];
                for (int i = start; i < end; i++) {
                    dosage[i - start] = Double.parseDouble(fields[i]);
                }
                dosageList.add(dosage);
            }
            double[][] dosageMatrix = new double[dosageList.size()][];
            dosageMatrix = (double[][]) dosageList.toArray(dosageMatrix);
            return dosageMatrix;
        }
    }

    public double[][] readAll() throws Exception {
        return readCols(1, Integer.MAX_VALUE);
    }
}

Changes:

Read the file only once
Removing unused fields
Bug fixes in array elements assignment
More specific exceptions

I have a number of suggestions:

Exceptions

I don't think your constructor actually throws Exception. Remove it?

Braces in if-blocks

Use braces even for one line if-blocks.

if(start < 1) {
    start = 1;
}
if(end > nCol)  {
    end = nCol;
}

Start parameter adjustment

The adjustment of start can be done before entering the while-loop. Making the while loop shorter and easier to understand. I suggest placing it at the beginning of the function, as you are adjusting an input parameter. Some people think it is bad practice to overwrite a parameter's value, but that depends on your personal taste. I think it is ok in this case, if done at the beginning of the function.

A different way to handle the situation of an incorrect input would be to throw an exception, because clearly the input is incorrect.

Documentation

I'd sugguest adding some comments to at least your public methods, so that the user of your utility function knows how to use it properly. This would also be a good place to document how you handle incorrect input (e.g. the correction of the start value to one, in case the entered value is smaller than 1).

Pattern as static variable

As the used pattern for splitting is a constant, you could handle it as such buy using a static attribute like this:

private static final Pattern splitPattern = Pattern.compile("\\s+");

This saves the jvm from compiling the pattern for each new instance of the Dosage class.

Micro optimization

You can save yourself the firstLine boolean by checking dosageList.isEmpty() instead.


Fixed array with configurable bounds A interesting challenge was brought to my attention: to "flip" an array. I thought it might be more convenient if its bounds were symmetrical, say [-N..N] instead of [0 .. 2N + 1].

Now I'm curious about what could be done to make this Array more mature. Say, like a Boost component.

#include <cassert>
#include <iostream>
#include <algorithm>

template <typename T, int LoBound, int HiBound> class Array {
  T arr_[HiBound - LoBound + 1];
public:
  Array()
  {
    std::fill(arr_, arr_ + sizeof(arr_) / sizeof(arr_[0]), T{});
  }
  Array(const Array& other)
    :arr_(other.arr_)
  {}
  Array(std::initializer_list<T> initList)
  {
    std::copy(initList.begin(), initList.end(), arr_);
  }
  T& operator[](int ix)
  {
    assert(ix >= LoBound && ix <= HiBound);
    return arr_[ix - LoBound];
  }
  const T& operator[](int ix) const
  {
    assert(ix >= LoBound && ix <= HiBound);
    return arr_[ix - LoBound];
  }
};

int main()
{
  enum {LoBound = -2, HiBound = 2};
  using A1D = Array<int, LoBound, HiBound>;
  Array<A1D, LoBound, HiBound> arr =
   { A1D{ 1,  2,  3,  4,  5},
     A1D{ 6,  7,  8,  9, 10},
    A1D{11, 12, 13, 14, 15},
    A1D{16, 17, 18, 19, 20},
    A1D{21, 22, 23, 24, 25} };
  for (int i = LoBound; i <= HiBound; ++i) {
    for (int j = LoBound; j <= HiBound; ++j) {
      std::cout << arr[-i][-j] << " ";
    }
    std::cout << "\n";
  }
  return 0;
}

It looks pretty clean to me.

The copy constructor doesn't have to be explicitly declared, you could = default it or just omit.
You shouldn't fill() the array. It is already default constructed, so that's just duplicated work. The default constructor again could be = default or omitted.
Actually, maybe could provide an explicit constructor that takes a fill value. That can be handy.
A size() method to return sizeof(arr_) / sizeof(arr_[0]) should be useful, for both internal and external uses.
Mark the class final? I'd probably do it, but that's probably a matter of personal preference.
If you're interested in making this class Standard compliant, well, there's a long road ahead ;). It needs iterators and several other methods ([c]begin/[c]end/front/back/at...). Look into std::array for the details.


OPOS DirectIO event review This may be a poor title name. So I'm basing what I did from this website.

The idea is though that I have made a lot of work to a Service Object for a OPOS MSR device. (useless detail) One of the DirectIO calls gets some data off of a card and was supposed to return a byte array. That ended up being a big mistake because Unicode and translating was just a big big BIG headache. So my suggestion was to just convert the byte array to a string in C++ then send fire a DirectIO event with said string and call it good. So I did that, and it appears to work. One thing that has me concerned though is one of the lines from that website

  The general rule to COM resource is, if you allocate it then you release it. The only exception is when a value is passed over a COM interface as an OUT param. In that case, the receiver of the value is responsible for releasing the resource

but I'm rather proud of my code none the less. I'll post my SO code and my mock up OPOS code (a test for me to show it works)

LONG ReaderHelper::ULCRead(LPVOID param)
{
    BYTE* bTmp = new BYTE[4];
    //a call to fill said bTmp with error checking and so forth
    CString str;
    for (int i = 0; i < 4; i++)
    {
        str.AppendFormat( _T("%02X"), bTmp[i]);
    }
    delete[] bTmp;
    BSTR bstring = str.AllocSysString();
    LONG data = str.GetLength();
    USNMSRRFID* pso = (USNMSRRFID*)param;
    COPOSMSR msr(pso->mlpDispatch);
    msr.SODirectIO(0, &data, &bstring);
    SysFreeString(bstring);
    msr.ReleaseDispatch();
}

MOCK UP OPOS
yes it is ugly, sorry

    void msr_DirectIOEvent(int EventNumber, ref int pData, ref string pString)
    {
        Console.WriteLine();
        Console.WriteLine("\t\t<{0}><{1}><{2}>:Len:{4}<{3}>", EventNumber, pData,pString, GetHexBytes(pString), pString.Length);
        Console.WriteLine();
    }
    string GetHexBytes(string str)
    {
        var bytes = System.Text.UnicodeEncoding.Unicode.GetBytes(str);
        string strbytes = "";
        foreach (byte b in bytes)
            strbytes += string.Format("{0}:", b.ToString("X2"));

        return strbytes;
    }

yields me this

2013-02-12.21:59:30 #Page:3#
2013-02-12.21:59:30     ULCRead(4)-00:00:00:00:
            <0><8><00000000>:Len:8<30:00:30:00:30:00:30:00:30:00:30:00:30:00:30:00:>
2013-02-12.21:59:30 #Page:4#
2013-02-12.21:59:30     ULCRead(4)-9D:5A:AD:47:
            <0><8><9D5AAD47>:Len:8<39:00:44:00:35:00:41:00:41:00:44:00:34:00:37:00:>
2013-02-12.21:59:30 #Page:5#
2013-02-12.21:59:30     ULCRead(4)-5D:E3:EC:93:
            <0><8><5DE3EC93>:Len:8<35:00:44:00:45:00:33:00:45:00:43:00:39:00:33:00:>

This BYTE* bTmp = new BYTE[4]; could just be put on the stack with BYTE bTmp[4]; (the proper term is actually automatic storage duration which in most cases means it ends up on the stack). This normally avoids a call to malloc and you don't have to delete it explicitly.
I'm not too versed in COM programming but if msr.SODirectIO(0, &data, &bstring); could throw an exception you might want to wrap it into a try {} finally {} in order to ensure the cleanup of the BSTR and COM resource is done regardless.
I know it's just a quick mockup but GetHexBytes could be reduced by using Linq:

string GetHexBytes(string str)
{
    var bytes = System.Text.UnicodeEncoding.Unicode.GetBytes(str);
    var hexBytes = bytes.Select(b => b.ToString("X2")).ToArray();
    return string.Join(":", hexBytes);
}


Project Euler # 44 Pentagon numbers in Python Pentagonal numbers are generated by the formula, \$P_n=\frac{n(3n−1)}{2}\$. The first ten pentagonal numbers are:
1, 5, 12, 22, 35, 51, 70, 92, 117, 145
It can be seen that \$P_4 + P_7 = 22 + 70 = 92 = P_8\$. However, their difference, \$70 − 22 = 48\$, is not pentagonal.
Find the pair of pentagonal numbers, \$P_j\$ and \$P_k\$, for which their sum and difference are pentagonal and \$D = |P_k − P_j|\$ is minimised; what is the value of D?

Awaiting feedback.
from time import time

def generate_pentagons(n):
    """generates next n pentagons"""
    pentagons = (num * (3 * num - 1) // 2 for num in range(1, n))
    for i in range(n - 1):
        yield next(pentagons)

def get_pentagons(n):
    """Assumes n is a range > 0.
    generates pentagons that obey to the + - rules."""
    pentagons = set(generate_pentagons(n))
    for pentagon1 in pentagons:
        for pentagon2 in pentagons:
            if pentagon1 + pentagon2 in pentagons and abs(pentagon1 - pentagon2) in pentagons:
                return pentagon1, pentagon2

if __name__ == '__main__':
    start_time = time()
    pent1, pent2 = get_pentagons(10000)
    print(abs(pent1 - pent2))
    print(f'Time: {time() - start_time} seconds.')

This is an answer focused primarily about styling

Your parameter names can be renamed to be more specific, like generate_pentagons(n) can be rewritten to generate_pentagons(number_of_pentagons). This change can also be implemented in the get_pentagons method. While this may seem useless in this program, since n can be easily interpreted as the number of pentagons to generate, it's a good practice to follow when you start writing large programs that require more specific parameter names.
Parameter constants should be UPPERCASE.
You don't use i in your loop in generate_pentagons method. To avoid generating an unused variable, you can use an underscore. The _ immediately signals the reader that the value is not important.

Final Code

from time import time

def generate_pentagons(number_of_pentagons):
    """generates next n pentagons"""
    pentagons = (num * (3 * num - 1) // 2 for num in range(1, number_of_pentagons))
    for _ in range(number_of_pentagons - 1):
        yield next(pentagons)

def get_pentagons(number_of_pentagons):
    """Assumes n is a range > 0. Generates pentagons that obey to the + - rules."""
    pentagons = set(generate_pentagons(number_of_pentagons))
    for pentagon1 in pentagons:
        for pentagon2 in pentagons:
            if pentagon1 + pentagon2 in pentagons and abs(pentagon1 - pentagon2) in pentagons:
                return pentagon1, pentagon2

if __name__ == '__main__':
    START_TIME = time()
    PENTAGON_1, PENTAGON_2 = get_pentagons(10000)
    print(abs(PENTAGON_1 - PENTAGON_2))
    print(f"Time: {time() - START_TIME} seconds.")


Search a directory structure for tar files and report on their size I have a complex directory structure on my web server that includes .tar files for download; I'm trying to create a report on the size distribution of these files (e.g. 10 are less than 1MB, two are 1-2MB, three are 2-3MB...)

Is there a better way to organize this code?  I started it as a console application, writing directly in the Main() function.  Now I'm taking a step back and thinking about improvements.

public class MyClass
{
    private static int counter = 0;
    private static Dictionary<int, int> sizeMap = new Dictionary<int, int>()
        {
              {0, 0}
            , {1, 0}
            , {2, 0}
            , {3, 0}
            , {4, 0}
            , {5, 0}
            , {6, 0}
            , {7, 0}
            , {8, 0}
            , {9, 0}
            , {10, 0}
            , {20, 0}
            , {30, 0}
            , {40, 0}
            , {50, 0}
            , {75, 0}
            , {100, 0}
            , {999, 0}
        };
    static void Main(string[] args)
    {
        DirectoryInfo cloud1 = new DirectoryInfo("C:\\brian");
        IEnumerable<DirectoryInfo> cloud1Dirs = cloud1.EnumerateDirectories();
        foreach (var dir in cloud1Dirs)
        {
            ParseDir(dir);
        }
        Console.WriteLine("==================================================");
        foreach (int i in sizeMap.Keys)
        {
            Console.WriteLine(i + "M\t" + sizeMap[i]);
        }
        Console.WriteLine("Press any key to exit");
        Console.ReadKey();
    }

    private static void ParseDir(DirectoryInfo currentDir)
    {
        if (counter++ > 1000)
        {
            return;
        }
        IEnumerable<FileInfo> tarFiles = currentDir.EnumerateFiles("*.tar");
        if (tarFiles.Count() > 0)
        {
            foreach (FileInfo tarFile in tarFiles)
            {
                int megLength = (int)(tarFile.Length / 1048576);
                int key = MakeDictionaryKey(megLength);
                sizeMap[key]++;
                Console.WriteLine(tarFile.Name + "\t" + megLength + "M");
            }
        }
        else
        {
            foreach (var dir in currentDir.EnumerateDirectories())
            {
                ParseDir(dir);
            }
        }
    }

    private static int MakeDictionaryKey(int n)
    {
        if (n <= 10) {
            return n;
        } else if (n <= 50) {
            return (int) Math.Ceiling(n / 10.0) * 10;
        } else if (n <= 75) {
            return 75;
        } else if (n <= 100) {
            return 100;
        } else {
            //Really large file!
            return 999;
        }
    }
}

The biggest complaint I would have with this code is that you're essentially defining your size map twice.  Imagine if you decide you don't want to report on files between 50 and 75MB - you remove the "75" entry from the dictionary, but forget to remove it from the MakeDictionaryKey method, and your program will occasionally fail at runtime.

First, I'd consider modifying your dictionary so instead of using "999" as a synonym for "really big file", you use int.MaxValue:

        , {100, 0}
        , {int.MaxValue, 0}

In your MakeDictionaryKey method, you're then looking for the smallest key value where the key value is greater than the file size you've provided to it.  With a bit of LINQ, that simply becomes:

private static int MakeDictionaryKey(int n)
{
   return sizeMap.Keys.Where(x => x <= n).Min();
}


Setup options for a jQuery plugin This code is the basic structure I use for coding plugins. It's pretty standard with the settings extended and the each loop.  However, now I'm working on the best way to provide setters/getters. For example, I may want to do something like:

var sp = $("#container").wScratchPad();

alert(sp.wScratchPad('size'));

sp.wScratchPad('color', '#FF0000');
sp.wScratchPad('reset');

I have my little setters/getters area below and this is what I have come up with so far. I'm looking for a nice clean standard way to do this for all future plugins, something I can just copy and paste for the most part at least. I'm looking for any input on this, particularly if there are anyways to do it cleaner and with less code.

(function($)
{
    var defaultSettings =
    {
        //settings here
    };

    $.fn.wScratchPad = function(option, settings)
    {
        //check for setter/getters
        if(typeof option === 'object')
        {
            settings = option;
        }
        else if(typeof option == 'string')
        {
            var sp = this.data('_wScratchPad');
            var hit = true;

            if(sp)
            {
                if(option == 'reset') sp.reset();
                else if(defaultSettings[option])
                {
                    if(settings) sp.settings[option] = settings;
                    else return sp.settings[option];
                }
                else hit = false;
            }
            else hit = false;

            return hit;
        }

        settings = $.extend({}, defaultSettings, settings || {});

        return this.each(function()
        {
            var elem = $(this);

            var sp = new ScratchPad(settings);

            //code here

            elem.data('wScratchPad', sp);
        });
    }

    //clasess/prototypes here
})(jQuery);

The only issue I see is with this:

else if(defaultSettings[option])

If you have any boolean options or numeric options which are set to 0, this will evaluate to false and the code within the if will not execute.

Perhaps use typeof instead.


Ruby: comparing two Arrays of Hashes and mapping one to the other Can you help me to optimize my code for the following problem:

An array of hashes named expected_tasks has tasks. Each task has a user that has to execute an action and contains fields.
The expected_tasks are related to the actual_tasks.
We want to execute the expected_task[action] on the actual_task["id]".
Therefor want to be able to select the actual_task["id"] where all the fields have the same value.

In the end this means that the expected_tasks[0] matches to the actual_tasks[0] because their fields are exactly the same. When their is a match, we can use actual_task["id"].

This is what the data looks like:

#the fields_to_check is an extraction of the keys of the expected_tasks minus user and action
fields_to_check = [ 'field_to_check_1', 'field_to_check_2' ]

expected_tasks = [{ 'user' => 'user',
                     'action' => 'action_y',
                     'field_to_check_1' => 'value_a',
                     'field_to_check_2' => 'value_a' },{
                     'user' => 'user',
                     'action' => 'action_x',
                     'field_to_check_1' => 'value_b',
                     'field_to_check_2' => 'value_b'} ]

sparse_actual_subtasks = [{ 'id' => '1' ,
                     'field_to_check_1' => 'value_b',
                     'field_to_check_2' => 'value_b' },{
                     'id' => '2' ,
                     'field_to_check_1' => 'value_a',
                     'field_to_check_2' => 'value_a' }]

The piece of code to map the id from actual_task to the right action + user from the expected_task:

expected_tasks.each do |expected_task|

  mapped_tasks = []

  actual_task = sparse_actual_subtasks.find do |actual_task|
    fields_to_check.all? do |field|
    expected_task[field] == actual_task[field]
    end
  end

  #map action and user to task with id
  actual_task['action'] = expected_task['action']
  actual_task['user'] = expected_task['user']
  mapped_tasks << actual_task

  puts mapped_tasks

end

def mapped_tasks(expected, subtasks, fields)
  fields_match = ->(t1, t2) {fields.all? {|f| t1[f] == t2[f]}}
  expected.map do |e|
    subtask = subtasks.find {|s| fields_match.(e,s)}
    subtask ? e.merge(subtask) : nil
  end.compact
end

should do the same as your original.

use merge rather than explicitly adding each field you want
create a lambda to name the condition of "all relevant fields matching".  this makes the loop within which it is called more readable.
rather than procedurally declaring an array and adding items to it, simply use map to chang all the expected items.  the ones that don't match are changed to nil, and we remove them at the end using compact

notes on the original code:

the data structures that we're assuming exist seem ill-fitted to the problem.  if they're within your control, consider simplifying them.  this should make the solution to the problem here more natural
avoid extraneous information in your variable names.  it gives the illusion of greater clarity but in fact makes the code harder to read at a glance.  eg fields is a clearer name than fields_to_check


Using method trait to return something in laravel model I have question about scope yesterday, and someone gives an answer to don't return anything in scope. Is return something in scope really bad practice?  Should I not return anything in the model except query?
Rather than scope, I want to create a normal method to return the select2 format. I have so many model/table that I need to return to select2 format, I think it's WET (write every time) if I must write every model or controller. I think I need to use DRY(Don't Repeat Yourself) principle, so I want to use a trait like below:
trait ModelTrait
{
    public static function getSelect2format($column = "name")
    {
        $data = static::select('id', $column.' as text')->orderBy($column, 'asc')->where($column, "like", "%".request()->q."%");

        if (is_array(request()->filter) && !empty(request()->filter)) {
            foreach(request()->filter as $key => $val)
            {
                if(in_array($key,$this->filterable))
                {
                    $data = $data->where($key, $val);
                }

            }
        }
        $data      = $data->limit(5)->get();

        return $data;
    }
}

I just need to call use ModelTrait in every model that I need and call it in the controller like below:
public function selectJson()
{

    $data = Customer::getSelect2Format("column_name");

    return \Response::json($data);
}

Is the code above break the best practice?
What are the concerns or something that I need to improve?

I have question about scope yesterday, and someone gives an answer to don't return anything in scope. Is return something in scope really bad practice?

This is because laravel is handling "it" for you.

getSelect2Format is not the best name since you are not handling formatting but apply a specific set of rules relevant to select2 js plugin.

I would suggest a repository class or a helper class to handle this task.

Your code is messy, very long lines too, which is hard to read! I would suggest following PSR-12.

I would suggest using response() or just return $data in controller class methods.

I would also suggest early returns in your methods to reduce nesting hence easy reviewing by reviewers.

suggest not using ->get() in a method unless you are absolutely certain on what method should do. i.e. you might want to apply a scope to this query  later on.

I suggest passing request data into the class to make it easier to test. Also, the less your class knows about other parts of the system the better.

Dont you have to use DB::raw for $column.' as text'?

class select2Filters
{
    public static function apply(
        Model $model,
        array $data,
        string $column = 'name',
        int $limit = 5
    ) {
        $filter = Arr::get($data, 'filter');

        return $model::selectRaw(['id', "{$column} as text"])
             ->where($column, 'like', "%{$data['q']}%")
             ->when(is_array($filter), function($query) use ($filter) {
                 $query->where(Arr::only($filter, $model::filterable));
             })
             ->orderBy($column, 'asc')
             ->limit($limit)
             ->get();
    }
}

I would also suggest adding an interface to each model that should have support for select2Filters. That interface should inforce filterable variable.
I would personally not use static, that way you can add configuration methods, i.e. setLimit(). OR use data struct class to contain data needed for select2Filters class.


Haskell Luhn Algorithm Trying to get to grips with Haskell. The problem is taken from here.

{-
Given Credit Card number: 49927398716

• Reverse the digits: 61789372994
• Sum the odd digits: 6 + 7 + 9 + 7 + 9 + 4 = 42 = s1
• The even digits: 1, 8, 3, 2, 9
•  Two times each even digit: 2, 16, 6, 4, 18
•  Sum the digits of each multiplication: 2, 7, 6, 4, 9
•  Sum the last: 2 + 7 + 6 + 4 + 9 = 28 = s2
• s1 + s2 = 70
• which, as it ends in zero, means that 49927398716 passes the Luhn test
-}

import Data.Char

data Validity = Valid | Invalid deriving Eq
type CardNumber = String

luhnCheck :: CardNumber -> Validity
luhnCheck s =
    let r = revDigits s
        (os,es) = oddsAndEvens r
        s1 = sum os
        s2 = sum $ map sumDigits $ map (*2) es
    in if (s1 + s2) `mod` 10 == 0 then Valid
       else Invalid

revDigits :: CardNumber -> [Int]
revDigits s = map digitToInt (reverse s)

oddsAndEvens :: [Int] -> ([Int],[Int])
oddsAndEvens is =
    let digitsWithIdx = zip is (cycle [1,2])
        odds = map fst $ filter (\d -> (snd d) == 1) digitsWithIdx
        evens = map fst $ filter (\d -> (snd d) == 2) digitsWithIdx
    in (odds,evens)

sumDigits:: Int -> Int
sumDigits i = sum $ map digitToInt $ show i

test :: Bool
test = let testData = ["49927398716", "49927398717", "1234567812345678","1234567812345670"]
    in  (map luhnCheck testData) == [Valid,Invalid,Invalid,Valid]

One thing that immediately pops out (and happens a lot with beginner Haskell programmers, IME):

s2 = sum $ map sumDigits $ map (*2) es

This means (reads as for me) "map (*2) on es, then map sumDigits on it, then sum the numbers."

What you typically want to do is to stay on the "function side" on things as long as possible:

s2 = sum . map sumDigits . map (*2) $ es

Meaning you compose as much as you can, and put only one application at the end. That makes it easier to eta-reduce and in general is considered more idiomatic.

You could also compose the mapped operation, bringing it down to classic "map-reduce" scenario (where "map" part is sumDigits . (*2), and "reduce" part is the sum).

s2 = sum . map (sumDigits . (*2)) $ es

I'd also flip the order of Valid and Invalid:

data Validity = Invalid | Valid deriving Eq

This way, when you derive Ord, Valid > Invalid, which is sometimes helpful and in general you'll see that it holds "intuitively", for Maybe (Just _ > Nothing), Either (Right _ > Left _) and similar values.


Hackerrank CTCI "Stacks: Balanced Brackets" Javascript Solution Here is the original problem, and below is my solution. I wonder what can be improved? Or is there a fundamentally better algorithm out there? Thank you! My intuition is telling me that it can be more concise and efficient than this.

  Input Format

  The first line contains a single integer, , denoting the number of
  strings.  Each line  of the  subsequent lines consists of a single
  string, , denoting a sequence of brackets.

  Output Format

  For each string, print whether or not the string of brackets is
  balanced on a new line. If the brackets are balanced, print YES;
  otherwise, print NO.

function main() {
    var t = parseInt(readLine());
    var i, isBalanced, stack, exp, len

    for (var a0 = 0; a0 < t; a0++) {
        exp = readLine().split('');

        stack = [];
        isBalanced = true;
        i = -1;
        len = exp.length;

        while (++i < len) {
            switch (exp[i]) {
                case "{":
                case "[":
                case "(":
                    stack.push(exp[i])
                    break;

                case "}":
                    if (stack.pop() !== "{") {
                        isBalanced = false
                    }
                    break;
                case "]":
                    if (stack.pop() !== "[") {
                        isBalanced = false
                    }
                    break;
                case ")":
                    if (stack.pop() !== "(") {
                        isBalanced = false
                    }
                    break;
                default:
            }

            if (isBalanced === false) {
                console.log("NO")
                break;
            }
        }

        if (isBalanced === true) {
            if (stack.length > 0) {
                if (stack[0] === "{" || stack[0] === "[" || stack[0] === "(") {
                    console.log("NO")
                }
            } else {
                console.log("YES");
            }
        }

    }
}

Simplify matching parentheses

These case statements are very similar, and take a couple of lines to write:

case "}":
    if (stack.pop() !== "{") {
        isBalanced = false
    }
    break;
case "]":
    if (stack.pop() !== "[") {
        isBalanced = false
    }
    break;
case ")":
    if (stack.pop() !== "(") {
        isBalanced = false
    }
    break;

You could simplify by reworking the case statements that handle the opening parentheses:

case "{":
    stack.push("}");
    break;
case "[":
    stack.push("]");
    break;
case "(":
    stack.push(")");
    break;

And then you could replace the handling of the closers with the simpler:

case "}":
case "]":
case ")":
    if (stack.pop() !== exp[i]) {
        isBalanced = false
    }
    break;

Avoid flag variables when possible

Instead of setting isBalanced = false and the multiple checks,
it would be simpler in such cases to immediately print the result and break, for example:

next:
for (var a0 = 0; a0 < t; a0++) {
    exp = readLine().split('');

    stack = [];
    i = -1;
    len = exp.length;

    while (++i < len) {
        switch (exp[i]) {
            case "{":
                stack.push("}");
                break;
            case "[":
                stack.push("]");
                break;
            case "(":
                stack.push(")");
                break;
            case "}":
            case "]":
            case ")":
                if (stack.pop() !== exp[i]) {
                    console.log("NO");
                    continue next;
                }
                break;
        }
    }

    console.log(stack.length == 0 ? "YES" : "NO");

Naming

What is exp? It's not very obvious, but actually it's characters.
How about calling it chars?


Google "Wiggle walk" I am doing Kickstart's problem Wiggle Walk. I have tried to code with matrix, dictionary and nested dictionary but the time exceeds the limit in each submition. Can anyone help me optimize it better or is there any algorithm which I may have overlooked.
The problem statement is

  Problem
  Banny has just bought a new programmable robot. Eager to test his coding skills, he has placed the robot in a grid of squares with R rows (numbered 1 to R from north to south) and C columns (numbered 1 to C from west to east). The square in row r and column c is denoted (r, c).

  Initially the robot starts in the square (SR, SC). Banny will give the robot N instructions. Each instruction is one of N, S, E or W, instructing the robot to move one square north, south, east or west respectively.

  If the robot moves into a square that it has been in before, the robot will continue moving in the same direction until it reaches a square that it has not been in before. Banny will never give the robot an instruction that will cause it to move out of the grid.
  Input
  The first line of the input gives the number of test cases, T. T test cases follow. Each test case starts with a line containing the five integers N, R, C, SR and SC, the number of instructions, the number of rows, the number of columns, the robot's starting row and starting column, respectively.

  Then, another line follows containing a single string of N characters; the i-th of these characters is the i-th instruction Banny gives the robot (one of N, S, E or W, as described above).

My code is as follows:

for j in range(int(input())):
instructions,rows,cols,startRow,startCol = map(int,input().split())
a=input()
dict={}
startRow-=1
startCol-=1
dict[startRow]={startCol:1}
i=0
while i <len(a):
    if a[i]=="N":
        if startRow-1 not in dict:
            dict[startRow-1]={startCol:1}
            i+=1
        elif startCol not in dict[startRow-1]:
            dict[startRow-1][startCol]=1
            i+=1
        startRow-=1

    elif a[i]=="S":
        if startRow+1 not in dict:
            dict[startRow+1]={startCol:1}
            i+=1
        elif startCol not in dict[startRow+1]:
            dict[startRow+1][startCol]=1
            i+=1
        startRow+=1

    elif a[i]=="W":
        if startRow not in dict:
            dict[startRow]={startCol-1:1}
            i+=1
        elif startCol-1 not in dict[startRow]:
            dict[startRow][startCol-1]=1
            i+=1
        startCol-=1
    elif a[i]=="E":
        if startRow not in dict:
            dict[startRow]={startCol+1:1}
            i+=1
        elif startCol+1 not in dict[startRow]:
            dict[startRow][startCol+1]=1
            i+=1
        startCol+=1

print("Case #{0}: {1} {2}".format(j+1,startRow+1,startCol+1))

At first glance, you code appears to have a lot of complicated and repetitive sections.  That's a bad smell, and often a hint that you should either split the repetitive parts off into a function or otherwise reorganize your code to get rid of the repetition.  (Sometimes, one option can be to just simplify the code until the repetitive parts become trivial.)

A lot of the complexity in the repetitive parts of your code seems to come from your decision to store the grid as a dictionary of dictionaries.  There's a couple of ways to simplify that:

In Python, dictionary keys can be tuples.  So you can just do:

visited = {}
# ...
if (row, col) not in visited:
    # do something
    visited[row, col] = True
else:
    # do something else

(Also, you shouldn't name your dictionary dict, both because it's the name of the dictionary type in Python and also because it's an uninformative name that doesn't explain what you're using the dictionary for.  Call its something meaningful like visited_cells or just visited instead.)
Also, since you're really only using the dictionary as a set, why not make it one?  The code can look exactly the same as above, except that you'll start with visited = set() instead of visited = {}, and mark cells as visited with visited.add((row, col)).
Alternatively, instead of using a dictionary or a set, you could just use a two-dimensional array (implemented e.g. as a list of lists) of booleans, initialized to all False, something like this:

visited = [[False] * cols for row in range(rows)]
# ...
if not visited[row][col]:
    # do something
    visited[row][col] = True
else:
    # do something else

I suspect this is what the exercise is actually expecting you to do.  However, I do rather like your idea of using a dictionary (or a set) instead, and I'm only mentioning this alternative for the sake of completeness.

One advantage of using a dictionary or a set to store the visited cells is that it means you don't actually need to care about the boundaries of the grid, since the dictionary can accommodate arbitrary large or small coordinates.  In particular, you don't actually need to adjust the input coordinates from one-based to zero-based indexing (and back again for output), so you can just eliminate those parts of your code.

Another way to reduce the repetitiveness of your code is to observe that all the sections for handling movement in different directions look basically the same, except for the index (row or column) being changed and the step size (+1 or -1) it's being changed by.  If you defined a dictionary that mapped the N/S/E/W direction letters to the corresponding coordinate offsets, something like this:

directions = {
    'N': (-1, 0),
    'S': (+1, 0),
    'W': (0, -1),
    'E': (0, +1),
}

then you could combine all those sections into a single one, something like this:

row = start_row
col = start_col
visited = {(row, col)}  # loop below assumes current square is always marked as visited

for compass_heading in instructions:
    row_step, col_step = directions[compass_heading]

    while (row, col) in visited:
        row += row_step
        col += col_step

    visited.add((row, col))

(Note that I renamed a few of your variables here.  You should not use variables named startRow and startCol to store the current row and column, since that's just plain misleading, and in any case variable names in Python should be written in snake_case instead of camelCase.  And a is just an awful variable name, so I renamed it to instructions.  You already have a variable named that, but it's completely unnecessary and unused.  You could just use the dummy variable _ in the list assignment instead.)

As for making the code faster, I see no obvious trivial opportunities there.  Using a single set instead of a dictionary of dictionaries is probably somewhat faster, perhaps even twice as fast if your running time is dominated by dictionary lookups, but not orders of magnitude faster.  A list of lists of booleans might be even faster for small grids, but the time to initialize it scales with the size of the grid, whereas the time to set up a single-element set (or dictionary) is basically constant.  Anyway, I'd suggest first focusing on cleaning up your code, and only worrying about optimization if it's still too slow after that.

If it is, a good starting point is to focus on worst-case performance, i.e. to think about how a sneaky test case designed might make your code run as slowly as possible, and try to find ways to counter that.

In particular, the worst case input for your algorithm (with or without the improvements I suggested above) is something like NSNSNSNSNSNSNSNSNSNSNSNSNS..., which forces the robot to move back and forth over an increasingly long stretch of visited cells, leading to an O(n²) execution time.

One way to avoid that would be to store a dictionary of "signposts" recording, for each move we make, where we ended up on the grid after that move.  That way, the next time the robot finds itself in the same square and moving in the same direction, it can skip all the cells in between.  Also, since the movement rules are symmetric, we can also leave behind "backwards" signposts recording where the robot came from as it entered each cell, allowing us to skip the intervening cells in the opposite direction as well.

A quick sketch of how to implement that might look something like this:

# look-up table of (row_step, col_step, opposite_heading) for each compass heading
directions = {
    'N': (-1, 0, 'S'),
    'S': (+1, 0, 'N'),
    'W': (0, -1, 'E'),
    'E': (0, +1, 'W'),
}

row = start_row
col = start_col
visited = {(row, col)}  # loop below assumes current square is always marked as visited
signposts = {}

for compass_heading in instructions:
    last_row = row
    last_col = col
    row_step, col_step, opposite_heading = directions[compass_heading]

    while (row, col) in visited:
        if (row, col, compass_heading) in signposts:
            # we've been this way before!
            row, col = signposts[row, col, compass_heading]
        row += row_step
        col += col_step

    visited.add((row, col))

    signposts[last_row, last_col, compass_heading] = (row, col)
    signposts[row, col, opposite_heading] = (last_row, last_col)

Note that, as written, the signposts dictionary always grows linearly with the size of the input.  You may obtain some additional speed and memory savings by removing old signposts as they become unnecessary.

In particular, we can safely remove any signposts we encounter as soon as we've used them, since they will either become useless or get overwritten anyway.  We can also safely remove any backwards signposts that we pass, since those also become useless.  With those changes, the optimized inner loop would look something like this:

    while (row, col) in visited:
        if (row, col, compass_heading) in signposts:
            # follow and remove signposts; any that are still valid will be restored below
            row, col = signposts.pop((row, col, compass_heading))
            del signposts[row, col, opposite_heading]
        row += row_step
        col += col_step

However, note that the set of visited cells will still grow linearly, and that meandering paths like, say, NESWNESWNESWNESWNESW... can still leave behind a linearly growing number of signposts, so the worst-case memory consumption is still linear even with this optimization.

Ps. Here's a simple online demo of the signpost-optimized algorithm I describe above, with some debug print statements added.  Feel free to play around with it to see how it works.


Utmost function that takes a comparator function I'm working through Paul Graham's "ANSI Common Lisp". In Chapter 6 (p105), he introduces the idea of utility functions. He points out these are core to bottom-up programming and reusability. He observes that for such utility functions in lisp and fp "it's easier to abstract out the general when you can pass the specific as a function argument".

The example below is given in the book (my own code for discussion will follow after this example).

 (defun most (fn lst)

 "What's the el of a list with highest score, according to provided
scoring fn?"

   (if (null lst)

      (values nil nil)
      (let* ((wins (car lst))           ; 'wins' is 'winning element'
             (max (funcall fn wins)))   ; 'max' is 'winning score'
        (dolist (obj (cdr lst))
          (let ((score (funcall fn obj)))
            (when (> score max)
              (setf wins obj
                    max score))))
        (values wins max))))

which runs as

> (most #'length '((a b) (a b c) (a)))
(A B C) ;
3

So, most above is effectively mapping the given list using the function argument then simply applying an arithmetic max to find the winner (though it does it in a slightly more efficient way than I just described).

I recalled a previous example where a comparator function was passed as an argument, allowing choice between > or <, for example. So, I added an second function argument to the above function as below (the only change is that the function arg has been added and then the single line where I've added a new comment).

(defun utmost (> fn lst)                ; note '>' is name of a variable!
  "Same as most above but with extra function argument allowing e.g. < or >"
  (if (null lst)
      (values nil nil)
      (let* ((wins (car lst))           ; 'wins' is 'winning element'
             (max (funcall fn wins)))   ; 'max' is 'winning score'
        (dolist (obj (cdr lst))
          (let ((score (funcall fn obj)))
            (when (funcall > score max) ; <- I replaced '>' with 'funcall >'
              (setf wins obj
                    max score))))
        (values wins max))))

which can be run as:

> (utmost #'< #'length '((a b) (a b c) (a)))
(A) ;
1

Above, we passed < as the new fn param, so 'utmost' has taken the form of returning the element of the given list of 'least' ('length') this time.

Questions

Given that this is my first attempt at 'designing' a function that takes more than one function parameter, I'm wondering if I'm on the right track at all.

Obviously the name 'utmost' is not very good ('extremal', 'outlier'? - don't like the latter).

I wonder if the more you abstract out and so the more general a function becomes, the harder it becomes to name it? :)

If I wanted to parameterise 'lowest' or 'highest' I feel there should be a better way to do it, because passing > or < doesn't seem as intuitive as being able to state you want the lowest or highest.

Are there any design errors or misconceptions worth pointing out in the above?

Are there some better examples of functions which use multiple function arguments worth making a part of ones toolkit?

I agree that this generalization is not particularly useful: to abstract a part of a program to a functional parameter is meaningful when you can reuse that program with a class of different functions, not with two operators like < and >. For this, it is sufficient to define a boolean parameter:

(defun most (fn lst &key lowest)
 "What's the el of a list with highest or lowest score, according to provided
scoring fn?"
  (if (null lst)
      (values nil nil)
      (let* ((wins (car lst))                   ; 'wins' is 'winning element'
             (val (funcall fn wins))            ; 'max' is 'winning score'
             (comparison (if lowest #'< #'>))) ; highest or lowest ?
        (dolist (obj (cdr lst))
          (let ((score (funcall fn obj)))
            (when (funcall comparison score val)
              (setf wins obj
                    val score))))
        (values wins val))))

CL-USER> (most #'length '((a b) (a b a c) (a)))
(A B A C)
4
CL-USER> (most #'length '((a b) (a b a c) (a)) :lowest t)
(A)
1

More, I think that in this case one possibility of generalization could be not on the comparison itself, but on the functional parameter applied to the results, that it the maximum (for which the comparison is used). In fact the Paul Graham's function is the optimization of the following simpler (and less efficient!) function:

(defun most (fn lst)
  (let ((val (reduce #'max lst :key fn)))
    (values (find val lst :key fn) val)))

Abstracting on the function #'max:

(defun most (fn op lst)
  (let ((val (reduce op lst :key fn)))
    (values (find val lst :key fn) val)))

Making it a little more efficient:

(defun most (fn op lst)
  (let* ((vals (mapcar fn lst))
         (val (reduce op vals))
         (wins (position val vals)))
    (values (when wins (nth wins lst)) val)))

But I think that even this generalization is not particularly useful, even if it could have a little more sense:

CL-USER> (most #'length #'max '((a b) (a b a c) (a)))
(A B A C)
4
CL-USER> (most #'length #'min '((a b) (a b a c) (a)))
(A)
1
CL-USER> (defun avg (a b) (/ (+ a b) 2))
AVG
CL-USER> (most #'length #'avg '((a b) (a b a c) (a)))
(A B)
2
CL-USER> (most #'length #'+ '((a b) (a b a c) (a)))
NIL
7


Good practices in an API Wrapper Class i'm trying to build an API wrapper class, that sends requests in a JSON format. the sample request format is something like this:

{
  "items": [
    {
      "brand": "brand",
      "idCategory": "idCategory",
      "idItem": "idItem",
      "attributes": [
        {
          "idUda": "idUda",
          "price": "price"
        },
        {
          "idUda": "idUda",
          "price": "price"
        }
      ],
      "description": "description",
      "skus": [
        {
          "price": {
            "offer": "offer",
            "default": "default"
          },
          "stock": {
            "available": "available",
            "amount": "amount"
          },
          "gtin": "gtin",
          "images": [
            "image",
            "image"
          ],
          "idSku": "idSku",
        }
      ]
    }
  ]
}

my first thought was to create an object for each element of the JSON request that is an array, something like this:

        $attr1 = new Attribute;
        $attr1->setIdUda('xxx')
        ->setPrice('99999');

        $price = new Price;
        $price->setOffer('teste')
        ->setDefault('99.99');

        $stock = new Stock;
        $stock->setAvailable(true)
        ->setAmount('9999');

        $product = new Product;
        $product->setBrand('Apple')
        ->setIdCategory('500')
        ->setIdItem('9999')
        ->setAttributes($attr1)
        ->setDescription('nice product')
        ->setSkus($price);
        ->setSkus($stock);

and each one these objects have a toArray() method, that returns an associative array, eg:

class Price {

        //getters and setters....

            function toArray() {
                return array(
                    'offer' => $this->offer,
                    'default' => $this->default
                )
            }
        }

inside of the Product class all these attributes setters are grouped to form an array so i can convert it to the JSON format later.

class Product {
        //.....
    public function setAttributes(Attribute $attributes) {

        $this->attributes['attributes'][] = $attributes->toArray();
                    return $this;
    }
}

but it still feels like it's highly coupled, because i have to set the key of each one of these arrays, in order to form the JSON data.

What would be the "best" approach to make this code easier to maintain? should i use classes only for some attributes? how should i consider it?

There is no problem modelling the request with classes representing each part of the request and a class that aggregates them.

One thing to note here, those classes adhere to builder pattern - builder of JSON.
Maybe you dont know that there is an interface \JsonSerializable which would comminucate the intent of the classes much better then a toArray() method. Although many arrays can be deemd json serializable, its not always true. Where as json serializable object is always serializable to json and it makes it clear that the json serializability is its purpose...

Then i quite dont understand this piece:

        $product
        ->setSkus($price);
        ->setSkus($stock);

You set skus to a Price object, then overwrite with Stock object?
Given this piece of data:

"skus": [
        {
          "price": {
            "offer": "offer",
            "default": "default"
          },
          "stock": {
            "available": "available",
            "amount": "amount"
          },
          "gtin": "gtin",
          "images": [
            "image",
            "image"
          ],
          "idSku": "idSku",
        }
      ]

I would expect that there is one more object between Product and Stock/Price object. Maybe a Sku object:

$product->addSku((new Sku())->setPrice($price)->setStock($stock));

And I wouldnt also see problem adding methods for more convenient work (that would instantiate the parts classes):

$sku = new Sku();
$sku->setPrice('offer', 'default');
$sku->setStock('available', 'amount')

instead of:

$sku = new Sku();
$sku->setPrice(new Price('offer', 'default'));
$sku->setStock(new Stock('available', 'amount'));

After all, those classes are not meant to represent the request, they represent a builder of a json request. And builder should make it as easy to build the target as possible....

EDIT: you were worried about coupling. I dont think this brings in any more coupling then necesary. The API uses specific keys in the request and these will never change, unless the API introduces BC break. But then maybe it was not a very good API structure... Anyway it wouldnt hurt to have those keys defined as constants of the bulder classes, so you dont have to repeat string literals all over the place... if nothing it makes it easier to find all occurences of a key.

Chances are that there are multiple ways the api could have been designed, yet retaining the same capabilities. And this means that there are also multiple ways to model the request with php objects. But you should prefer to have this 1:1. The api designer was probably aware of the multiple ways and chosen the one he chosen for a reason. And you should follow that reasoning...

Well this statement might have seem quite abstract, so let me complete it with an example. Lets say we have api structure:

{
  "scopeA": {
    "configA1": "valueA1"
  },
  "scopeB": {
    "configB1": "valueB1"
  }
}

but we can also model it as this structure:

{
  "scopeAconfigA1": "valueA1",
  "scopeBconfigB1": "valueB1"
}

but the first structure is the API structure, and so the PHP model should also have the first structure. Simply following the same reasoning why the api designers chose the first structure. Whatever reasoning it may have been...


Reverse and Add: find a palindrome This is one of codeeval challenges

  Challenge Description:

  Credits: Programming Challenges by Steven S. Skiena and Miguel A.
  Revilla

  The problem is as follows: choose a number, reverse its digits and add
  it to the original. If the sum is not a palindrome (which means, it is
  not the same number from left to right and right to left), repeat this
  procedure. E.g.

  195 (initial number) + 591 (reverse of initial number) = 786

  786 + 687 = 1473

  1473 + 3741 = 5214

  5214 + 4125 = 9339 (palindrome) In this particular case the palindrome
  9339 appeared after the 4th addition. This method leads to palindromes
  in a few step for almost all of the integers. But there are
  interesting exceptions. 196 is the first number for which no
  palindrome has been found. It is not proven though, that there is no
  such a palindrome.

Here is my solution for it:

#!/usr/bin/env python
import sys

"""
I've done some tests with timeit and it seems that both numeric and string version
have the same performance (at least for numbers < 10000)

# def reverse_num(num):
#   rev = 0
#   while(num > 0):
#     rev = (10*rev)+num%10
#     num //= 10
#   return rev
"""

def reverse(num):
    """Reverses the number

       >>> reverse(1456)
       6541
       >>> reverse(111)
       111
    """
    return int(str(num)[::-1])

def palindrome(num):
    """Return the number of iterations required to compute a palindrome

       >>> palindrome(195)
       (4, 9339)
    """
    # compute in 100 iterations or less
    for i in range(100):
        rnum = reverse(num)
        if rnum == num:
            break
        num = num + rnum
    return (i, num)

if __name__ == "__main__":
    with open(sys.argv[1]) as f:
        for line in f:
            print "%s %s" % palindrome(int(line))

Any remarks on the code style, on the algorithm itself?

Only 2 things:

Why do you have this? This makes your code a bit unclear and confusing. Remove this if you do not want to use it as it might make your actual code look messy.

# def reverse_num(num):
#   rev = 0
#   while(num > 0):
#     rev = (10*rev)+num%10
#     num //= 10
#   return rev

It is discouraged to use for i in range(100): in Python.


Constructing a lexicon and frequency array from a long string I have a dataset X_train, which is an array where each entry is an email (a string of characters). There are 11,314 emails, each of which is about 500 characters long. (X_train is a processed version of the training data in the newsgroups dataset.)

Ultimately, my goal is to build from scratch a tf-idf function (knowledge of which is probably not necessary for answering my question). To get there, I have constructed a lexicon which contains each unique word in X_train once and only once. My lexicon has 211441 elements. I also need an array where each entry frequency_train[i] is the number of emails in which a given term lexicon_train[i] appears.

I construct the frequency array as follows:

frequency_train = np.zeros(211441)
for i in range(211441):
    count = 0
    for email in X_train:
        if lexicon_train[i] in email:
           count = count + 1
    frequency_train[i] = count

In the same cell, I am also doing something similar with the testing data X_test. I've been running this in Jupyter notebook, and this process takes a while. A previous and very similar task took about 90 minutes. I suspect that I'm doing this task the slowest possible way. Is there a faster way of doing this? I would also welcome answers that explain why this process should take a long time.

For each word in the lexicon you are searching through each email: (11,314 emails) * (60 words/email) * (211441 word lexicon) = lots of comparisons.

Flip it around.  Use collections.Counter.  Get the unique words in each email (use a set()) and then and update the counter.

from collections import Counter

counts = Counter()

for email in x_train:
    words = set(email.split())   # <= or whatever you use to parse the words
    counts.update(words)

This will give you a dict mapping words in the emails to the number of emails they are in.  (11,314 emails) * (60 words/email) = a lot fewer loops.
This probably also recreated the lexicon (e.g.
counter.keys() should be the lexicon.

On my computer, it takes 7 seconds to generate 115000 random 60-word emails and collect the counts.


An object for passing a chain of errors in JavaScript I want to pass errors in errors in order to know exactly the chain of causes, but without augmenting or even reading any stack until this becomes really necessary. And this is how I would achieve this:

class RError extends Error {
    constructor(options = {}) {
        super();
        this.name = options.name;
        this.message = options.message;
        this.cause = options.cause;
    }
}

Example usage:

try {
    throw new RError({
        name: 'FOO',
        message: 'Something went wrong.',
        cause: new RError({
            name: 'BAR',
            message: 'I messed up.'
        })
    });
} catch (err) {
    console.error(err);
}

Here is the result:

{ FOO: Something went wrong.
    at RError (/Users/boris/Workspace/playground/index.js:5:9)
    at Object.<anonymous> (/Users/boris/Workspace/playground/index.js:13:11)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Function.Module.runMain (module.js:575:10)
    at startup (node.js:160:18)
    at node.js:456:3 name: 'FOO', message: 'Something went wrong.', cause:
   { BAR: I messed up.
       at RError (/Users/boris/Workspace/playground/index.js:5:9)
       at Object.<anonymous> (/Users/boris/Workspace/playground/index.js:16:16)
       at Module._compile (module.js:541:32)
       at Object.Module._extensions..js (module.js:550:10)
       at Module.load (module.js:458:32)
       at tryModuleLoad (module.js:417:12)
       at Function.Module._load (module.js:409:3)
       at Function.Module.runMain (module.js:575:10)
       at startup (node.js:160:18)
       at node.js:456:3 name: 'BAR', message: 'I messed up.', cause: undefined } }

Since I couldn’t find anyone on the web who uses this technique, I’m wondering if there is any drawback to this approach.

Update: I’ve created two fiddles, an es5 and an es6 version (run, see results in the console).

es5 version: https://jsfiddle.net/borisdiakur/vkj9e9fx/3/
es6 version: https://jsfiddle.net/borisdiakur/3mtfwqv1/1/

Update: I’ve created a module: https://github.com/borisdiakur/rerror

Cosmetically speaking, you can use the ES6 destructuring operator to make your arguments a bit more readable:

class RError extends Error {
    constructor({name, message, cause}) {
        super(name, message);
        this.cause = cause;
    }
}

As for drawbacks, the only one I can think of is having to do it yourself instead of having the language do it for you (Like, in example, in Java, which is where I guess this is coming from). So you'll need to remember to do it every time you have a rethrown exception. Personally, I wouldn't find this acceptable, as I would surely forget.