Finding line numbers of duplicate lines in a log file I have a log file with a million lines, and my task is to code something that outputs the lines that have duplicates, and the line numbers.

I've thought of two ways to approach this :

1) Use python's inner tools :

load my file's lines in a list,
load them in a dict in which the keys are the lines, and the values are the number of times they appear
for each element of my dict where value >= 2, output the element and the indexes of this element in my list

Here's my code :

def find_dupl(log):
    # put in list
    with open(log) as l:
        liste = l.readlines()

    # put in dict
    dico = dict()
    for i in liste:
        dico[i] = dico.get(i, 0) + 1

    output_dict = {}
    for i in dico:
        if dico[i] > 1: # for dico's element where value >= 2
#            print(i,        # print the element
#                dico[i],    # how many times it appears
#                [a+1 for a, b in enumerate(liste) if b == i] # the lines where it appears
#                )
            output_dict[i] = [a+1 for a, b in enumerate(liste) if b == i]
    return(output_dict)

2) Go bruteforce, which is something like this :

load my file's lines in a list
for each element i of the list
check all the elements j after i
if i == j, output i, i's index and j's index

Option 1's code works fine and is fast (like 1 second on my (pretty average) computer), so my job is done and I haven't written the code for option 2. But I'm being curious on which one would be faster : I can see that option 2 is something like O(n^2) (or am I mistaken ?), but as I don't know the inner workings of lists and dicts (I didn't major in CS), I'm not really able to tell the O() of option 1.

I'm also curious about if there would be an even faster way (without modules) ?

To go through your programs:

Load the file into a list - \$O(n)\$ time and memory.
Go through the list and add to a dict - \$O(n)\$ time and memory.
Loop through the new dict: \$O(n)\$ time and memory,
if the item has duplicates, get the lines: \$O(n)\$ time, \$O(1)\$ memory.

In total \$O(n^2)\$ time, \$O(n)\$ memory.

And so it's \$O(n^2)\$ time, \$O(n)\$ memory.

Load the file into a list - \$O(n)\$ time and memory.
Loop through the file - \$O(n)\$ time, \$O(1)\$ memory.
Loop through the file for second index - \$O(n)\$ time and \$O(1)\$ memory.

In total \$O(n^2)\$ time, \$O(n)\$ memory.

It's \$O(n^2)\$ time, and \$O(n)\$ memory.

And so I'd change your program to remove the second loop through the file, to force this you can remove changing the file to a list.
You want to use what you are in (1), but to store a list of all the file indexes. After this you then want to filter all items that don't have a length greater than 1.
And so you should be able to get something like:

from collections import defaultdict

def find_dupl(log):
    output = defaultdict(list)
    with open(log) as f:
        for i, line in enumerate(f):
            output[line].append(i)
    return {k: v for k, v in output.items() if len(v) > 1}

This has \$O(n)\$ time and \$O(n)\$ memory. Yes it does use a module, but it's built in and makes your code much faster, and simpler.


TCP chat room in Python I have recently started looking into networking with Python. I saw some tutorials about the socket library so I decided to give a "live chat room" idea a go. It works (on Windows, os.system("cls") may be problematic) but was just wanting to see how people could improve upon it

server.py

from socketserver import ThreadingTCPServer, BaseRequestHandler
from threading import Thread
import pickle,time,datetime,os

messages = []
temp = []
os.system("cls")
class Echo(BaseRequestHandler):

    def handle(self):
        self.temp = []
        Thread(target=self.send).start()
        self.username = self.request.recv(8192)
        self.username = self.username.decode()
        print("Got connection from {}:{}".format(self.client_address[0],
                                                 self.client_address[1]))

        while True:
            msg = self.request.recv(8192)
            msg = "[{} {}]: {}".format(datetime.datetime.now().strftime("%H:%M:%S"),
                               self.username,
                               msg.decode())

            messages.append(msg)
            print(msg)

            if not msg:
                break

    def send(self):

        global temp, messages
        while 1:

            if len(self.temp) != len(messages):

                data_string = pickle.dumps(messages)
                self.request.send(data_string)
                self.temp = [item for item in messages]
if __name__ == "__main__":
    serv = ThreadingTCPServer(("",20000), Echo)
    serv.serve_forever()

client.py

import socket,pickle,os
from threading import Thread
import time
s = socket.socket()
s.connect(('localhost',20000))
def receive():

    while True:

        data = s.recv(8192)
        data = pickle.loads(data)
        os.system("cls")
       for item in data:
            print(item)

username = input("Enter your username: ")
Thread(target=receive).start()

s.send(username.encode())
time.sleep(0.1)
while True:

    msg = input(">>")

    if not msg:
       break
    s.send(msg.encode())
    time.sleep(0.1)

So... handle() gets an incoming connection. Great! What's not so great is that it then spawns self.send in a thread that will never die.

More minorly, I'm not sure why you'd do

    self.username = self.request.recv(8192)
    self.username = self.username.decode()

instead of just

    self.username = self.request.recv(8192).decode()


Vector-transposing function I profiled a library I'm writing that uses vector transposes and found that I am spending a good bit of time doing the following transpose. I am using a std::vector of std::vector<double>s to represent the column vectors.

What are some ways to optimize this function?

std::vector<double> transpose_vector(const std::vector<std::vector<double>> &column_vec) {
  // take a column vector:
  // |x1|
  // |x2|
  // |x3|
  // and return a row vector |x1, x2, x3|
  std::vector<double> row_vector;
  for (auto c : column_vec) {
    for (auto r : c) {
      row_vector.push_back(r);
    }
  }
  return row_vector;
}

Change your column loop to use a reference.

for (const auto &c : column_vec)

Without the reference, a copy will be made of each vector.  This will involve a memory allocation.  Using the reference you avoid all that, which should save a good deal of time since each c will be a single element vector.

auto r can stay since r will be a double.

Combine this with using reserve on row_vector will eliminate all but one memory allocation.


Swing "Number Guesser" program I have made a UI that guesses the number that the user is thinking of(0 - 100), and I am wondering if it can be improved(I guess this is simple AI). I tried to shorten it as best I could, but it still seems a bit "fluffed." I would greatly appreciate anyone willing to help me shorten and/or improve it.  It has 3 buttons, a "Too High", "Too Low", and "Just Right" button, and the guess is displayed in the center of the window. I have posted an image below also.

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.EventQueue;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.UIManager;

public class NumberGuesser extends JFrame {
    private static final long serialVersionUID = 1L;
    private Random rand = new Random();
    private int min = 0;
    private int max = 100;
    private int guess = rand.nextInt(max);
    private JLabel guesses;
    private Font font = new Font("Times New Roman", Font.BOLD, 30);

    public NumberGuesser() {
        initGUI();
        setTitle("Number Guesser");
        pack();
        setLocationRelativeTo(null);
        setVisible(true);
        setResizable(false);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
    }

    private void initGUI() {
        JLabel title = new JLabel(" I Guess Your Number(0-100) ");
        title.setFont(font);
        title.setHorizontalAlignment(JLabel.CENTER);
        title.setBackground(Color.BLACK);
        title.setForeground(Color.WHITE);
        title.setOpaque(true);
        add(title, BorderLayout.NORTH);

        guesses = new JLabel(Integer.toString(guess));
        guesses.setFont(font);
        guesses.setHorizontalAlignment(JLabel.CENTER);
        add(guesses, BorderLayout.CENTER);

        JPanel buttons = new JPanel();
        add(buttons, BorderLayout.SOUTH);

        JButton higher = new JButton("Too High");
        higher.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                max = guess;
                guessAgain(min, max);
            }
        });
        buttons.add(higher);

        JButton correct = new JButton("Just Right");
        correct.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                guesses.setText("I WIN!");
            }
        });
        buttons.add(correct);

        JButton lower = new JButton("Too Low");
        lower.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                min = guess;
                guessAgain(min, max);
            }
        });
        buttons.add(lower);
    }

    private void guessAgain(int min, int max) {
        guess = min + (int)(Math.random() * ((max - min) + 1));
        guesses.setText(Integer.toString(guess));

    }

    public static void main(String[] args) {
        try {
            String laf = UIManager.getCrossPlatformLookAndFeelClassName();
            UIManager.setLookAndFeel(laf);
        }
        catch (Exception e) {}

        EventQueue.invokeLater(new Runnable(){
            public void run() {
                new NumberGuesser();
            }
        });
    }
}

The output looks like this:

Thanks for sharing the code!

pros

Naming

Finding good names is the hardest part in programming. You did a good job here.
Especially I'd like to mention:

you followed the Java Naming Conventions
Your identifier names come from the problem domain.

Visibility

You declared all your members and methods private.
This is good since it serves both, the information hiding/encapsulation principle and the interface segregation principle.

cons

Magic numbers

Your code contains some literal values.

You should create constants with descriptive names for them. e.g.:

private static final int INITIAL_MINIMUM = 0;
private static final int INITIAL_MAXIMUM = 100;
private static final int TITLE_FONT_SIZE = 30;
private static final String TITLE_FONT_NAME = "Times New Roman";

Inheritance

Your NumberGuesser extends JFrame. I know that many examples out there do this but it is wrong. Your class does not add or change any behavior of its base class. It only configures it. This could also be done with an explicitly created JFrame instance.

Only extend a base class when you have to add or change the base classes behavior.

Call of JFrame.setVisible()

This call should always be the last after completely configuring the frame.

Separation of Concerns / Single Responsibility Pattern

Each class or method in a program should have a clear responsibility.
The instantiation of objects (using the new operator) is a responsibility of its own as well as the configuration of the GUI elements.

Work in constructor

In general constructors should only initialize (final) object members with values passed in as parameters.

Yes, many online resources and even official tutorials do initialization in constructors, but it is still a bad practice

Members

Member variables define the objects state. But some of your members will never change. That means they will never get a new value during life time of the object.

You should document that by making them final.


Implementing a generic and covariant Set in Scala I'm struggling with manually implementing a simple purely functional Set that's also generic and covariant on its type parameter:

Set must be a trait, allowing for multiple implementations.
it must be covariant on the type of the element it contains.
an implementation's type must not be lost when calling methods that return a new Set.

The first two points are fairly trivial and simply mean that the trait should look something like:

trait Set[+A] {
  def isEmpty: Boolean
  def insert[B >: A](b: B)(implicit order: Ordering[B]): Set[B]
  def contains[B >: A](b: B)(implicit order: Ordering[B]): Boolean
}

The problem with this implementation is that insert, for example, will always return an instance of Set. If I were to write a specialised implementation of Set with useful helper methods, its type would be lost as soon as an element was inserted in it.

This is what I've come up with, which seems to work but I still find confusing and can't help but feel is over-complicated (recursive higher kinded types!):

trait Set[+A, Repr[+X] <: Set[X, Repr]] {
  this: Repr[A] =>

  def isEmpty: Boolean
  def insert[B >: A](b: B)(implicit order: Ordering[B]): Repr[B]
  def contains[B >: A](b: B)(implicit order: Ordering[B]): Boolean
}

Here's a sample implementation using a simple binary tree:

sealed trait CustomSet[+A] extends Set[A, CustomSet]

case class Node[A](value: A, left: CustomSet[A], right: CustomSet[A]) extends CustomSet[A] {
  override def isEmpty = false

  override def insert[B >: A](b: B)(implicit order: Ordering[B]) =
    if(order.lt(b, value))      Node(value, left.insert(b), right)
    else if(order.gt(b, value)) Node(value, left, right.insert(b))
    else                        this

  override def contains[B >: A](b: B)(implicit order: Ordering[B]) =
    if(order.lt(b, value))      left.contains(b)
    else if(order.gt(b, value)) right.contains(b)
    else                        true
}

case object Leaf extends CustomSet[Nothing] {
  override def isEmpty                                        = true
  override def insert[B](b: B)(implicit order: Ordering[B])   = Node(b, Leaf, Leaf)
  override def contains[B](b: B)(implicit order: Ordering[B]) = false
}

The questions that I have are:

I don't believe the self-type is strictly necessary here. Are there good reasons to leave it? to remove it?
is this the correct / best / simplest way to implement what I'm trying to write? Are there any alternatives?

The this: Repr[A] is not absolutely necessary, but it you omit it, you're encoding a sort of weird type constraint:
that the class is not necessarily a Repr, but its insert[B] must produce a Repr[B] (who's insert[C] produces a Repr[C], and so on).
I can't think of a realistic situation where having or not having the self-type would prevent or lead to a bug, so IMO it doesn't actually matter much.
Your recursive higher-kinded type signature is absolutely fine, and in fact very closely mirrors that of SetLike from the standard collection library.

There is at least one alternative: making it a typeclass. Here's what it might look like:

trait CovariantSet[S[+_]] {
  def isEmpty[A](xs: S[A]): Boolean
  def contains[A, B >: A](b: B)(xs: S[A])(implicit order: Ordering[B]): Boolean
  def insert[A, B >: A](b: B)(xs: S[A])(implicit order: Ordering[B]): S[B]
}

sealed trait CustomSet[+A]
case object Leaf extends CustomSet[Nothing]
case class Node[+A](value: A, left: CustomSet[A], right: CustomSet[A]) extends CustomSet[A]

object CustomSet {
  implicit val customSetIsCovariantSet: CovariantSet[CustomSet] = new CovariantSet[CustomSet] {
    override def isEmpty[A](xs: CustomSet[A]): Boolean = xs == Leaf

    override def insert[A, B >: A](b: B)
                                  (xs: CustomSet[A])
                                  (implicit order: Ordering[B]): CustomSet[B] = xs match {
      case Leaf => Node(b, Leaf, Leaf)
      case Node(v, l, r) => order.compare(b, v) match {
        case 0 => xs
        case i if i < 0 => Node(v, insert(b)(l), r)
        case _ => Node(v, l, insert(b)(r))
      }
    }

    @tailrec
    override def contains[A, B >: A](b: B)
                                    (xs: CustomSet[A])
                                    (implicit order: Ordering[B]): Boolean = xs match {
      case Leaf => false
      case Node(v, l, r) => order.compare(b, v) match {
        case 0 => true
        case i if i < 0 => contains(b)(l)
        case _ => contains(b)(r)
      }
    }
  }
}

The idea is that the data structure (the CustomSet) can be defined separately from the evidence that it has an instance for the CovariantSet typeclass (customSetIsCovariantSet). Whether or not you think it's simpler is a matter of opinion, but it does at least avoid the recursive type.

You can then enrich CovariantSets for convenience:

object CovariantSet {
  implicit class Ops[S[+_], A](xs: S[A])(implicit ev: CovariantSet[S]) {
    def isEmpty: Boolean = ev.isEmpty(xs)
    def contains[B >: A](b: B)(implicit order: Ordering[B]): Boolean = ev.contains(b)(xs)
    def insert[B >: A](b: B)(implicit order: Ordering[B]): S[B] = ev.insert(b)(xs)
  }
}

I would also probably make the Node constructor private, so users can't construct a malformed (out of order) tree.

Your larger and more fundamental design problem is that the data structure doesn't make sense unless the Ordering is fixed (or at least its compare operation doesn't change its mind about values already in the Set).

Here are a couple problematic examples of where this can go wrong:

Leaf.insert(1).insert(2).contains(2) // true. great, working as expected

Leaf.insert(1).insert(2).contains(2)(Ordering.Int.reverse) // false. your data structure is broken

Leaf.insert(1).insert(2)(Ordering.Int.reverse).contains(2) // false. your data structure is broken

There are a couple ways around this I can think of, the first more practical than the second:

Implement a covariant HashSet instead. Every object in Scala has a hashCode: Int and an equals(other: Any): Boolean method, which is all you need.
Store an Ordering[A] in the Set. Whenever you implicitly pass in a new Ordering[B], also implicitly pass in an object that proves that the new Ordering[B] agrees with every decision the old Ordering[A] has already made.


Adding a Node to the Linked List is taking a longer Time when using a reference to the Tail Node I am trying to add two numbers in the form of linked List and return their result in a Linked List as given in https://leetcode.com/problems/add-two-numbers/

Question:

  You are given two non-empty linked lists representing two non-negative
  integers. The digits are stored in reverse order and each of their
  nodes contain a single digit. Add the two numbers and return it as a
  linked list.

  You may assume the two numbers do not contain any leading zero, except
  the number 0 itself.

Example:

Input: (2 -> 4 -> 3) + (5 -> 6 -> 4)
Output: 7 -> 0 -> 8
Explanation: 342 + 465 = 807, my solution has solved all the test cases but after refactoring, it is taking longer than the original code

Solution 1:

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

public class Solution {
    public ListNode AddTwoNumbers(ListNode l1, ListNode l2) {
        ListNode l3 = null;
        int carry = 0;
        while (l1 != null || l2 != null) {
            int first = 0,
            second = 0;
            if (l1 != null) {
                first = l1.val;
                l1 = l1.next;
            }
            if (l2 != null) {
                second = l2.val;
                l2 = l2.next;
            }
            int Digit = first + second;
            if (carry != 0) {
                Digit = Digit + carry;
                carry = 0;
            }
            if (Digit > 9) {
                carry = Digit / 10;
                Digit = Digit % 10;
            }
            AddLastNode(Digit, ref l3);
        }
        if (carry != 0) {
            AddLastNode(carry, ref l3);
            carry = 0;
        }
        return l3;
    }

    /// In here I am looping through the Linked List every time,to find the tail node
    private static void AddLastNode(int Digit, ref ListNode l3) {
        if (l3 != null) {
            AddLastNode(Digit, ref l3.next);
        }
        else {
            l3 = new ListNode(Digit);
        }
    }
}

So  to avoid looping through all the Nodes,in the below solution I am using a reference for the Tail Node

Solution 2:

public class Solution {
    public ListNode AddTwoNumbers(ListNode l1, ListNode l2) {
        ListNode l3 = null;
        ListNode tailNode = null;
        int remainder = 0;
        while (l1 != null || l2 != null) {
            int sum = 0;
            if (l1 != null) {
                sum = l1.val;
                l1 = l1.next;
            }
            if (l2 != null) {
                sum += l2.val;
                l2 = l2.next;
            }
            if (remainder != 0) {
                sum += remainder;
            }
            if (sum > 9) {
                remainder = sum / 10;
                sum = sum % 10;
            }
            else {
                remainder = 0;
            }
            ///In here I am using tailNode has reference for adding new node to the end of Linked List
            if (tailNode == null) {
                l3 = new ListNode(sum);
                tailNode = l3;
            }
            else {
                tailNode.next = new ListNode(sum);
                tailNode = tailNode.next;
            }
        }
        if (remainder != 0) {
            tailNode.next = new ListNode(remainder);
        }
        return l3;
    }
}

Since I got a tail node for the end of Linked List instead of going through entire Linked List,I thought the solution 2 will have better performance.But it is still taking more time to execute than the first solution ,Any code changes would be appreciated

Solution 1 is taking 108 ms to execute while Solution 2 is taking 140 ms

In terms of the times on LeetCode, they can be dependent upon the server load as much as the code.  I tried your code and got a range of times from 100 ms to 136 ms.  Holding onto the last node is a better solution than repeatedly trying to find it but on a list of only a few nodes I don't know how much impact it would have.

In terms of the code, this strikes me as a good place to use the Null Condtional Operator (?.).  We can get around the repeated null check on the tailnode by using a dummy head for the return list.

public ListNode AddTwoNumbers(ListNode l1, ListNode l2)
{
    ListNode l3 = new ListNode(-1);  // dummy head
    ListNode tailNode = l3;
    int remainder = 0;
    while (l1 != null || l2 != null) {

        var sum = (l1?.val ?? 0) +
                  (l2?.val ?? 0) +
                  remainder;
        l1 = l1?.next;
        l2 = l2?.next;

        if (sum > 9) {
            remainder = sum / 10;
            sum = sum % 10;
        }
        else {
            remainder = 0;
        }
       tailNode.next = new ListNode(sum);
       tailNode = tailNode.next;

    }
    if (remainder != 0) {
        tailNode.next = new ListNode(remainder);
    }
    return l3.next; // skip the dummy head when returning
}


Mapping properties between two Objects of different structure Something I've found myself having to do reasonably often is map properties from an Object (usually a heavily nested MongoDB document) to a different 'shape' to then pass it to a function call or API endpoint etc. Combine this with a development environment in which these 'shapes' are prone to change and it felt messy to do the mapping manually each time (though learning object destructuring often made it much neater!).

I've written this code to accept two arrays containing key names, an object and a delimiter option. The function then returns an object containing the properties pointed to by the keys in the input array, but with the key names as specified in the output array.

const transposeObjectByArraysOfPairs = (input, output, object, delimiter = '.') => {
    const toReturn = {};

    input.map((keyString, idx) => {
        const keysIn = keyString.split(delimiter);

        // copy object to avoid mutating original
        let _currentProp = Object.assign({}, object);

        for (let i = 0; i < keysIn.length; i++) {
            _currentProp = _currentProp[keysIn[i]];
        }

        const keysOut = output[idx].split(delimiter);

        let len = keysOut.length;

        let _obj = {[keysOut[len - 1]]: _currentProp};

        for (let j = len - 2 ; j >= 0; --j) {
            _obj = {[keysOut[j]]: _obj};
        }

        Object.assign(toReturn, _obj);
    });

    return toReturn;
};

const object = {a : {b: { c: 10, d: 30 }}};

const input = ['a.b.c', 'a.b.d'];
const output = ['A.B', 'B.C'];

console.log(misc.transposeObjectByArraysOfPairs(input, output, object));
// { A: { B: 10 }, B: { C: 30 } }

Whilst this function has neatened up chunks of my code significantly, I can't help but feel there's a neater way to do this. Iterating through an array of keys to get to the desired property feels quite hacky in particular.

In a perfect world I think I'd essentially have a function that mapped from one object schema to another and then I could simply maintain schema and know that if I made changes, I wouldn't need to modify the code that did the mapping.

Maybe I don't understand the question exactly, but what's stopping you from just creating a function to do the mapping, like this:

function obj1ToObj2(obj1){
    return {
        A: {
            B: obj1.a.b.c
        },
        B: {
            C: obj1.a.b.d
        }
    }
}


Backbone.js Dependency Injection API Design I'm writing a dependency injection plugin for the Backbone javascript framework, and I'm not sure what the API for constructor argument injection should look like.

Just for reference, Backbone objects are defined as follows:

var LoginView = Backbone.View.extend({
  //constructor
  initialize: function(options) {
    //define an instance property
    this.loginProvider = options.loginProvider;
  },

  //method
  authenticate: function() { },

  //prototypal property (available via instance, shared between all instances)
  navigation: App.navigationService
});

My plugin supports instance property injection as follows:

var LoginView = Backbone.View.extend({
  //...
  loginProvider: needs.property('LoginProvider')
});

var loginView = new LoginView();
loginView.loginProvider.doLogin(); //loginProvider is auto-injected

I'm quite happy with this, but I also want to support constructor argument injection. Here I have run into doubt regarding best way forward. Here are the options I've considered:

Add class method inject

var LoginView = Backbone.View.extend({
  initialize: function(options, bar) {
    //property "foo" will be injected to options
    //second argument "bar" will be injected
  }
}).inject({foo:'Foo'}, 'Bar');

This is clean, but it "hides" the injection to the bottom of the class definition. I feel the constructor injection configuration should be somewhere near the constructor, at the very least.
Wrap the initialize method

var LoginView = Backbone.View.extend({
  initialize: needs.args({foo:'Foo'}, 'Bar', function(options, bar) {
    //property "foo" will be injected to options
    //second argument "bar" will be injected
  }
});

Now the injected arguments are right next to the constructor definition, but this gets a bit messy when you have more than one or two dependencies:

var LoginView = Backbone.View.extend({
  initialize: needs.args(
    { foo:'Foo', bar:'Bar' },
    'BazService',
    'QuxFactory',
    function(options, baz, qux) {
      //...
    })
});

This method also lies to the user, because the arguments are not actually injected to the initialize method directly. Instead they are injected into the class constructor function, which in turn passes them to initialize.
Define a "convention-based" property injectArgs, which is used, if present

var LoginView = Backbone.View.extend({

  injectArgs: [{foo:'Foo'}, 'Bar'],

  initialize: function(options, bar) {
    //property "foo" will be injected to options
    //second argument "bar" will be injected
  }
});

This solves problems posed by options 1 and 2, but it feels... icky. Because the property is defined on the class prototype, it will continue to hang around on every instance of the object even after initialization and I don't think it has any business being there.
None of the above

Got a better idea? I want to keep the plugin unobtrusive, so there are two maxima I want to keep to: No modifying Object prototype, and the Backbone objects must be creatable by extend, so no factory tricks.

If I had to pick, I would pick the second option.  It's the clearest to me and aligns well with how the plugin injects properties. I don't think it's "lying" to the user because it's obvious that the needs.args function is doing something with the arguments before passing back control to the actual initialize function. But like you said, long arg lists will be annoying.

My only other note would be that I'm unsure how useful injection is in a dynamic language like javascript. I've used in statically typed languages like Java where it's definitely useful, but never entertained the idea in js. I'd definitely be curious to see the final result.

Edit: After some research there looks like another alternative - reflection: http://merrickchristensen.com/articles/javascript-dependency-injection.html


Build SQL query criteria from form input I am using a <form> to send various input parameters as a GET to retrieve data from a very simple two table MySQL db.

After filtering the inputs (resulting in $inputs), I'm using the following code to build the WHERE clause which I've adapted from a MATLAB function we use to build XQueries.

It works fine, but I'm just curious if there's a better way to do this in PHP, or if there is common practice for doing such.

public function parseInvoiceConstraints($inputs) {

    //pulled from different location but literals for this example
    $invTable = "invoices"
    $custTable = "customers"

    //used for queries
    $constraints = "";
    $sql_conj = "WHERE ";

    //parseargs into SQL WHERE
    foreach ($inputs as $param => $val) {
        if ($val == false) {
            continue;
        }
        switch ($param) {
            case 'inv_num':
                $constraints = "$constraints"
                        . "$sql_conj"
                        . "$invTable.$param=$val";
                $sql_conj = " AND ";
                break;
            case 'name':
                $constraints = "$constraints"
                        . "$sql_conj"
                        . "$custTable.$param LIKE \"%$val%\"";
                $sql_conj = " AND ";
                break;
            case 'customer_id':
                $constraints = $constraints
                        . "$sql_conj"
                        . "$invTable.$param=$val";
                $sql_conj = " AND ";
                break;
            case 'pcs':
                $constraints = $constraints
                        . "$sql_conj"
                        . "$invTable.$param=$val";
                $sql_conj = " AND ";
                break;
            case 'tag_num':
                $constraints = $constraints
                        . "$sql_conj"
                        . "$invTable.$param=$val";
                $sql_conj = " AND ";
                break;
        }
    }
    return $constraints;
}

For the URL:

/invoice.php?inv_num=&name=gon&customer_id=&pcs=2&tag_num=72

the function will return:

WHERE customers.name LIKE "%gon%" AND invoices.pcs=2 AND invoices.tag_num=72

If it works, it's good but it's not very easy to understand. It is a Loop-switch_sequence and can be seen as an anti-patern. See:
https://en.wikipedia.org/wiki/Loop-switch_sequence

If this is working, don't touch it, but if you have to simplify it, you can get rid of the loop and the switch and have just blocs like this.

if(!empty($inputs['inv_num']){
    if(!empty($constraints)){
       $constraints .= ' AND ';
    }
    $constraints .= "$invTable.$param='$val'";
}
//....... other variables name, customer_id, pcs ...
$constraints = $sql_conj . ' ' . $constraints;


Delete temp files on startup I wrote some code for my question about deleting temp files that weren't deleted on exit before and would like to get some feedback if I forgot some details or got something wrong.

private static final String PROJECTNAME = "projectname";
private static final String LOCKNAME = "lock_ThisIsAVeryLongFilenameNoOtherApplicationWillUse.lock";

private static void initTempDir(Path tempPath) throws IOException {
    final Path tempFolder = Files.createTempDirectory(tempPath, PROJECTNAME);
    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
        try {
            DeepDelete.deepDelete(tempFolder);
        } catch (IOException e) {
            e.printStackTrace();
        }
    }));

    Timer timer = new Timer(true);
    timer.scheduleAtFixedRate(new TimerTask() {

        @Override
        public void run() {
            Path lockFile = tempFolder.resolve(LOCKNAME);
            try {
                Files.write(lockFile, ByteBuffer.allocate(Long.BYTES).putLong(System.currentTimeMillis()).array());
            } catch (IOException e) {
                System.err.println("Failed to secure avaliability of temp folder, exiting!");
                e.printStackTrace();
                System.exit(-1);
            }
        }

    }, 60_000, 60_000);
    // Make first write manually to be sure the file has been written when
    // old temp files get deleted. Otherwise, the current temp file not
    // having a time stamp (yet) would be classified as old and removed.
    Files.write(tempFolder.resolve(LOCKNAME),
            ByteBuffer.allocate(Long.BYTES).putLong(System.currentTimeMillis()).array());

    long currentTime = System.currentTimeMillis();
    Files.list(tempFolder.getParent())//
            .filter(Files::isDirectory)//
            .filter(p -> p.getFileName().toString().startsWith(PROJECTNAME))//
            .filter(p -> {
                Path lock = p.resolve(LOCKNAME);
                if (!Files.isRegularFile(lock))
                    return true;
                try {
                    return ByteBuffer.wrap(Files.readAllBytes(lock)).getLong() < currentTime - 60_000 * 5;
                } catch (IOException e) {
                    e.printStackTrace();
                    return false;
                }
            })//
            .forEach(p -> {
                try {
                    DeepDelete.deepDelete(p);
                } catch (IOException e) {
                    e.printStackTrace();
                }
            });
}

The method takes in the path to the default temp directory and first initializes the own temp folder with the hardcoded prefix PROJECTNAME. The folder has a file whose name is in the constant LOCKNAME in it. Each minute, the current time is written in this file. In the end of initialization, each folder in the temp directory with the same prefix and with a lock file older than five minutes is considered a leftover from a crash and gets deleted.

The code looks good and should work properly. Here is some feedback nevertheless.
I don't really understand why the file is named as a "lock". A lock file is typically used to prevent multiple instances of a program to run, or synchronise acess to a resource between different processes.

Usage of Timer

The Timer was effectivelly replaces by Executors, more specifically ScheduledThreadPoolExecutor as mentioned in the javadoc of Timer. Oneimmediate advantage is that you can submit tasks as Runnable with a lambda expression.

Deletion of tempFolder

Make sure that the DeepDelete.deepDelete(tempFolder); does not fail if tempFolder doesn't exist. If the application terminates due to an error (such as the failure to write to tempFolder, you would have a second exception.

Duplicated code

Extract the following to a method, which appears twice. It will also do always the error handling:

   private static void writeFile(Path tempFolder) {
    try {
        Path lockFile = tempFolder.resolve(LOCKNAME);
        Files.write(lockFile, ByteBuffer.allocate(Long.BYTES).putLong(System.currentTimeMillis()).array());
    } catch (IOException e) {
        System.err.println("Failed to secure avaliability of temp folder, exiting!");
        e.printStackTrace();
        System.exit(-1);
    }
}

Hardcoded values

Define hardcoded values in constants. It will be easier to maintain.

private static final Integer DELAY_MS = 60_000;
private static final Integer DELETE_THRESHOLD_MS = 5 * 60_000;

Read safely

ByteBuffer.wrap(Files.readAllBytes(lock)).getLong() < currentTime - 60_000 * 5;

The entire file will be read to memory and only then the first Long.BYTES bytes are read as a long. This could be a problem if the file happens to be very large, even though it's not expected.
It would be better to only read the first Long.BYTES of the file.

Inconsistency?

Looking at

if (!Files.isRegularFile(lock))
  return true;
try {
  return ByteBuffer.wrap(Files.readAllBytes(lock)).getLong() < currentTime - 60_000 * 5;
} catch (IOException e) {
  e.printStackTrace();
  return false;
}

There seems to be an inconsistency. If the file doesn't exist, the directory is kept. If the file exists but its content is unexpected, we delete the directory. Both situations represent a corrupted temporary folder of the program and it seems to me that both situations should have the same handling.


Extracting XML data from a node in Common Lisp (Closure XML DOM) This is a function intended to take an XML element (in closure XML format) and a name and find a piece of simple data by that name (either in a tag or attribute) within that XML element. It works, but I'm still new to LISP - can it be written more elegantly?

(defun find-data-inside-xml (element name)
  ;; Maybe there's an attribute?
  (if (dom:has-attribute element name) (dom:get-attribute element name)
  ;; There isn't, we have to do it the ugly way. Find suitable subnodes.
    (let ((candidate-values (loop for node across (dom:child-nodes element)
                           when (string= (dom:node-name node) name) collect node)))
     ;; Check there was only one of them.
     (cond
        ((> (length candidate-values) 1) (error "XML element has multiple ~S elements." name))
        ((= (length candidate-values) 0) (error "XML element missing ~S element." name))
        (t (let ((cand-children (dom:child-nodes (car candidate-values))))
            ;; Check the one there was, has only one child node.
            (cond
             ((> (length cand-children) 1) (error "XML ~S element has complex content." name))
             ((= (length cand-children) 0) (error "~S element has no content." name))
             (t (let ((content-node (elt cand-children 0)))
                 ;; If that child node is text, that's our value. Otherwise, error.
                 (if (eq (dom:node-type content-node) :text)
                     (dom:node-value content-node)
                     (error "~S element value is not text." name)))))))))))

you can use REMOVE instead of the LOOP
you can replace the CONDs and IFs and ERRORs with ASSERT
replace CAR with FIRST

The specific functions make the intent of the code a bit more clear.

Untested:

(defun find-data-inside-xml (element name)
  (if (dom:has-attribute element name)
      (dom:get-attribute element name)
    (let* ((candidates (remove name (dom:child-nodes element)
                               :key #'dom:node-name
                               :test-not #'string=))
           (n-candidates (length candidates)))
      (assert (not (> n-candidates 1)) ()
              "XML element has multiple ~S elements." name)
      (assert (not (= n-candidates 0)) ()
              "XML element missing ~S element."       name)
      (let* ((candidate-children (dom:child-nodes (first candidate-values)))
             (n-candidate-children (length candidate-children)))
        (assert (not (> n-candidate-children 1)) ()
                "XML ~S element has complex content." name)
        (assert (not (= n-candidate-children 0)) ()
                "~S element has no content."          name)
        (let ((content-node (elt candidate-children 0)))
          (assert (eq (dom:node-type content-node) :text) ()
                   "~S element value is not text." name)
          (dom:node-value content-node))))))

Sometimes one also sees a bit macrology, to make the intent even clearer on the code level. But that has other costs:

(defmacro with-length-checked ((varn var &rest exprs) &body body)
  (let ((asserts (loop for (test test-a msg . args) in exprs
                       collect `(assert (not (,test ,var ,test-a)) ,msg ,@args))))
    `(let ((,var (length ,varn)))
       ,@asserts
       ,@body)))

(defun find-data-inside-xml (element name)
  (if (dom:has-attribute element name)
      (dom:get-attribute element name)
    (let ((candidates (remove name (dom:child-nodes element)
                              :key #'dom:node-name
                              :test-not #'string=)))
      (with-length-checked (candidates n-candidates
                            (> 1 "XML element has multiple ~S elements." name)
                            (= 0 "XML element missing ~S element."       name))
        (let ((candidate-children   (dom:child-nodes (first candidate-values))))
          (with-length-checked (candidate-children n-candidate-children
                                (> 1 "XML ~S element has complex content." name)
                                (= 0  "~S element has no content."         name))
             (let ((content-node (elt candidate-children 0)))
               (assert (eq (dom:node-type content-node) :text) ()
                 "~S element value is not text." name)
               (dom:node-value content-node))))))))


HTTP handlers exercise (tour of go) The challenge, from Tour of Go, "Exercise: HTTP Handlers":

  Implement the following types and define ServeHTTP methods on them. Register them to handle specific paths in your web server.

type String string

type Struct struct {
    Greeting string
    Punct    string
    Who      string
}

For example, you should be able to register handlers using:

http.Handle("/string", String("I'm a frayed knot."))
http.Handle("/struct", &Struct{"Hello", ":", "Gophers!"})

Solution:

package main

import (
    "log"
    "net/http"
    "fmt"
)

type String string

type Struct struct {
    Greeting    string
    Punct       string
    Who         string
}

func (s String) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    if _, err := fmt.Fprint(w, s); err != nil {
        log.Fatal("Error in ServerHTTP of String.")
    }
}

func (s *Struct) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    if _, err := fmt.Fprint(w, s.Greeting, s.Punct, s.Who); err != nil {
        log.Fatal("Error in ServerHTTP of String.")
    }
}

func main() {
    http.Handle("/string", String("I'm a frayed knot."))
    http.Handle("/struct", &Struct{"Hello", ":", "Gophers!"})
    log.Fatal(http.ListenAndServe("localhost:4000", nil))
}

Any suggestions on how to improve this code?

The logic of trying to write or else log.Fatal is duplicated in the two ServeHTTP implementations. And the log message looks copy pasted, because it's the same for both the String and *Struct versions, for example:

func (s *Struct) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    if _, err := fmt.Fprint(w, s.Greeting, s.Punct, s.Who); err != nil {
        log.Fatal("Error in ServerHTTP of String.")
    }
}

You could extract the common logic to a function, for example:

func writeOrElseLogFatal(writer func() (int, error), msg string) {
    if _, err := writer(); err != nil {
        log.Fatalf("Error in ServeHTTP of %s", msg)
    }
}

And then call it like this, for example for *Struct:

func (s *Struct) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    writer := func() (int, error) {
        return fmt.Fprint(w, s.Greeting, s.Punct, s.Who)
    }
    writeOrElseLogFatal(writer, "Struct")
}

Or you could make things simpler by not logging at all in these functions:

func (s String) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    fmt.Fprint(w, s)
}

func (s *Struct) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    fmt.Fprint(w, s.Greeting, s.Punct, s.Who)
}


Multiple dispatch decorator in Python I've written a decorator for easily creating multiple dispatch functions:

from functools import wraps

def multi_dispatch(for_function):
    """
    Returns a multiple dispatch version of the function.

    The returned function doesn't allow keyword arguments.

    >>> @multi_dispatch
    ... def foo(a, b):
    ...     pass
    ...
    >>> @foo.add
    ... def _foo(a: int, b: int, c : str = '33'):
    ...     return a + b*3 + len(c)
    ...
    >>> @foo.add
    ... def _foo(a: int, b: int, c = 3):
    ...     return a + b*3 - c
    ...
    >>> @foo.add
    ... def _foo(a: float, b: float):
    ...     return a*2 - b
    ...
    >>> foo(4, 5)
    16
    >>> foo(1.0, 2.0)
    0.0
    >>> foo(4, 5, 'loooong')
    26
    >>> foo(3, 4.5)
    Traceback (most recent call last):
      ...
    KeyError: (<class 'int'>, <class 'float'>)
    """
    @wraps(for_function)
    def decorated(*args):
        return decorated.registry[tuple(type(i) for i in args)](*args)

    decorated.registry = {}

    def adder(func):
        """
        Adds the supplied function to the multiple dispatch registry.
        """
        code = func.__code__
        args = code.co_varnames
        annotations = func.__annotations__
        types = tuple(annotations[a] for a in args if a in annotations)
        decorated.registry[types] = func
        return func

    decorated.add = adder
    return decorated

I'd like to read your comments about the code (efficiency, re-usability, etc.), documentation and everything else that you feel is worth noting.

Update: After fixing all the points listed in the choosen answer, I have created a new version.

You have a docstring and some doctests, which is great! The docstring needs some work, though, as described below.
The docstring needs to mention that the returned function has an add method, and explain what this method does.
The docstring needs to explain how the multiple dispatch works: that is, by exact match on the types of the arguments against the types of the annotations of the function instances. You ought to make it clear that arguments without annotations are ignored by the dispatcher.
The docstring says, "The returned function doesn't allow keyword arguments" but you might want to elaborate on exactly what this means.
The example code in the docstring is not very motivating. Why would I want to want to write a function foo like this? It would be very helpful to the reader if you could come up with an example that more closely resembles a real use case.
A group of multi-dispatch functions is a persistent data structure with two methods (add and __call__) and so would be clearer, I think, if it were implemented as a class.
The introspection code would be much clearer if you used inspect.signature. (This was introduced in 3.3, but since you're using function annotations, you probably don't mind this.)
The function that is decorated with @multi_dispatch is only used for its name, module, and docstring (the items that are copied by functools.update_wrapper). The function body is ignored. This seems like a waste: why not add this function to the registry too?
If you wrote return decorated instead of return func at the end of adder, then you wouldn't have to respell the names of the method instances.

Update: What I mean by this is that you have to spell your method instances _foo rather than foo, to avoid overwriting the multi-method in the variable foo. With my suggestion, you wouldn't have to do this.
Dispatching on the exact types of the arguments limits the usefulness of multiple dispatch. In languages like Common Lisp that have multiple dispatch, dispatching can be performed on subclass matches (not just on exact matches). This makes for more natural fallback code, for example:

@find.add
def find(needle: str, haystack: str):
    # When both arguments are strings, Python has a built-in efficient
    # search operation.
    return haystack.find(needle)

@find.add
def find(needle: Sequence, haystack: Sequence):
    # Otherwise, for generic sequences, use Boyer–Moore–Horspool.
    h = len(haystack)
    n = len(needle)
    skip = {needle[i]: n - i - 1 for i in range(n - 1)}
    i = n - 1
    while i < h:
        for j in range(n):
            if haystack[i - j] != needle[-j - 1]:
                i += skip.get(haystack[i], n)
                break
        else:
            return i - n + 1
    return -1

With your implementation one would have to provide separate method instances for (list, list), (tuple, tuple), (list, tuple) and other combinations of sequence types.

Update: In comments you express concern about the speed of doing the subclass lookups. But you can deal with that by maintaining a cache of the actual argument types and the method instance that was found corresponding to those types. The revised dispatch logic would be something like this:

arg_types = tuple(type(i) for i in args)
try:
    method = cache[arg_types]
except KeyError:
    method = ... # slow subclass dispatch logic here
    cache[arg_types] = method
return method(*args)

(When adding a new method instance, this might invalidate some of the cached lookups, so you'd need to clear the cache in the add method.)
You probably want to look at Guido van Rossum's multimethod implementation for other ideas (like "stacking" of multimethod decorators).


99 bottles of beer on the wall I'm super new to this so please be nice. I tried solving Chris Pine's challenge to print the entire lyrics to 99 bottles of beer on the wall. This seems to work, but is there a better/more efficient way to do it? Just want to learn how to improve it if I can.

var1 = 99
loop do
    puts (var1.to_s) + ' bottles of beer on the wall, ' + (var1.to_s) + ' bottles of beer, '
    puts 'take one down, pass it around, ' + ((var1 - 1).to_s) + ' bottles of beer on the wall.'
    var1 = (var1 - 1)
break if var1 == 1
end

In any programming language you should always use meaningful names for your variables, functions, methods, and whatever else you need to assign a name.
In your case, var1 is completely non-descriptive. A better name would be bottles, bottle_count or bottles_remaining to be even more specific.
Secondly, in Ruby, you rarely need to use a plain loop. In your case, you basically want to repeat roughly the same thing 99 times (well actually, no, you don't exactly, but we'll get to that later).
A simple way to do this in Ruby is to say:
99.downto(1) do |remaining|
  # stuff to do 99 times
end

The remaining variable will start at 99 and count down to 1, so we can use it to print the verses.
And speaking of printing, you're making things difficult for yourself by not using string interpolation.
If you use double quotes instead of single quotes, you can use #{...} to insert stuff into strings. Like so:
99.downto(1) do |remaining|
  puts "#{remaining} bottles of beer on the wall, #{remaining} bottles of beer."
  puts "Take one down and pass it around, #{remaining-1} bottles of beer on the wall."
  puts # print a blank line between verses.
end

Note that #{...} can contain any Ruby code (but keep it simple for your own sake!), so a bit of arithmetic to subtract 1 works too. And there's no need to use to_s everywhere, as that happens automatically when using string interpolation.
The above code is pretty much equivalent to your current code, except that it prints a blank line between verses. And the fact that it prints 99 verses - you're actually only printing 98 verses in your version.
... However, it's printing wrong lyrics. Grammatically incorrect lyrics, at any rate.
Most verses do indeed go like this:

X bottles of beer on the wall, X bottles of beer.
Take one down and pass it around, (X-1) bottles of beer on the wall.

where X is whatever number of bottles we have left. (Note, by the way, how much this looks like the Ruby code above; the code's almost as simple to read.)
But notice the changes in the last 3 verses:

2 bottles of beer on the wall, 2 bottles of beer.
Take one down and pass it around, 1 bottle of beer on the wall.
1 bottle of beer on the wall, 1 bottle of beer.
Take it down and pass it around, no more bottles of beer on the wall.
No more bottles of beer on the wall, no more bottles of beer.
Go to the store and buy some more, 99 bottles of beer on the wall.

And that's why it's a programming challenge. If it was just about writing those 5 lines of code above, it wouldn't be too tricky.
But what's interesting is when you have exceptions to the rules, and those last 3 verses are just that. They don't even follow a single, different set of rules - they're each different in their own way.
How you go about handling that is up to you, though. It's a challenge, after all. There are clever ways, simple ways, complex ways, nonsensical ways... which one you pick is up to you. The idea is to make you consider the problem and its potential solutions; there isn't a single correct answer. The destination is already given (print the lyrics), so it's all about the journey.

Other small stuff:

The Ruby convention is to use 2 spaces of indentation - not 4 spaces, not tabs.
The break line should be indented.
The parentheses in var1 = (var1 - 1) are completely unnecessary. Furthermore, you can write it as var1 -= 1, which is a shorthand for the same thing.


Preorder, inorder, postorder for tree in C++11 Please review my code for preorder, inorder and postorder of a tree. I am using C++11 (or wanted to use), so let me know if anything is deviating from the C++11 standard.

#include <iostream>
#include <memory>

using std::cout;
using std::endl;
using std::shared_ptr;

struct node
{
    int data;
    shared_ptr<node> left;
    shared_ptr<node> right;
};

shared_ptr<node> newNode (int i)
{
    shared_ptr<node> n (new node);
    n->data = i;
    n->left = nullptr;
    n->right = nullptr;

    return n;
}

void preOrder(shared_ptr<node> n)
{
    if(n == nullptr)
        return;

    cout<<n->data<<" ";

    preOrder((n->left));
    preOrder((n->right));
}

void inOrder(shared_ptr<node> n)
{
    if(n == nullptr)
        return;

    inOrder((n->left));

    cout<<n->data<<" ";

    inOrder((n->right));
}

void postOrder(shared_ptr<node> n)
{
    if(n == nullptr)
        return;

    postOrder((n->left));
    postOrder((n->right));

    cout<<n->data<<" ";

}

int main(int argc, char* argv[])
{
    shared_ptr<node> r = newNode(1);
     (r->left) = newNode(2);
     (r->right) = newNode(3);
    (r->left->left) = newNode(4);
    (r->left->right) = newNode(5);

    preOrder(r);
    cout<<endl;

    inOrder(r);
    cout<<endl;

    postOrder(r);
    cout<<endl;

    return 0;
}

Memory management

Don't think that is the appropriate smart pointer.

shared_ptr<node> newNode (int i)

Do you have plans to actually share ownership of a node? Or is it more likely that ownership will be done by the patent of the node? I would have gone with unique_ptr<node> if I was going to use smart pointers.

make_X

You should not be using new with smart pointers. There are make_XXX commands that combine all the memory allocation required into a single operation (thus making it more efficient).

   std::shared_ptr<node> n  = std::make_shared<node>(val);  // or make_unique

Using

I suppose this is better than using namespace std;

using std::cout;
using std::endl;
using std::shared_ptr;

But its still pretty sloppy and lazy. Pull those declarations into the tightest scope possible (ie inside a function).

Braces

This looks strange

preOrder((n->left));
preOrder((n->right));

Now I am all for using braces to make expression easy to read or to force a particular evaluation order. But this just looks strange and does not help in the evaluation.

Prefer '\n'

Prefer '\n' to std::endl.

The difference between the two is that std::endl forces a flush after placing the new line on the stream. It is very rare that you actually want to force a flush (as the runtime is much better at making that decision than you). Excessive flushing just makes the stream libraries slow.

“\n” or '\n' or std::endl to std::cout?
Why does endl get used as a synonym for “\n” even though it incurs significant performance penalties?


First Unique Character in Java by frequency I have completed an implementation for finding the first unique character.
Is there any way I can improve it?
package com.datastructures;

public class FirstUniqueCharacter {

    public static void main(String args[]){

        String string = "neslihan";
        System.out.println(firstUniqueChar(string));
    }

    private static int firstUniqueChar(String string) {

        int freq[] = new int[26];

        for(int i=0; i< string.length() ;i++){
            freq[string.charAt(i) - 'a'] ++;
        }
        for (int j =0;j<string.length();j++){
            ;
            System.out.println(string.charAt(j)-0);
            System.out.println('a'-0);
            if(freq[string.charAt(j) - 'a'] == 1) {
                return j;
            }
        }
        return -1;
    }
}

As from @RobAu 's comment the title is misleading because you returns the index of the first unique character in your frequencies array so that the index 0 refers to the 'a' char, 1 refers to the 'b' char, etc., making the assumption that your string just contains only letter chars coerent with the ascii table.
In your code you make use of string methods length() and charAt to iterate over the string , you can iterate directly over one char array like below:
private static int firstUniqueChar(String string) {
    int freq[] = new int[26];
    char[] arr = string.toCharArray();

    for(char c : arr) { ++freq[c - 'a']; }

    //other instructions
}

You are using subtraction by 0 in these lines:
System.out.println(string.charAt(j)-0);
System.out.println('a'-0);

I presume you are using this subtraction because you want to print the int value of the char, you can solve this issue casting the char value to int. Below my version of your code:
private static int firstUniqueChar(String string) {
    int freq[] = new int[26];
    char[] arr = string.toCharArray();

    for(char c : arr){
        ++freq[c - 'a'];
    }

    for (int j =0;j < arr.length; ++j){

        System.out.println((int) arr[j]);
        System.out.println((int) 'a');

        if(freq[arr[j] - 'a'] == 1) {
            return j;
        }
    }

    return -1;
}

From your code I have the impression at the moment you are mixing different concepts in an unclear manner.


Mask latitude text field I have an input element on one of the pages. This input is supposed to take a string of digits from a user. In order to enhance UX I'd like to mask input's value at any given time. Like so:

What came up with as a solution is the following piece of code:

(function () {
  var input = document.querySelector('input');

  var state = '';

  input.addEventListener('keyup', function (e) {
    if ( e.key.match(/\d/) )     state += e.key;
    if ( e.key === 'Backspace' ) state = state.slice(0, -1);

    var degree = state.slice(0,2),
        minute = state.slice(2,4),
        second = state.slice(4,6);

    if (degree) degree += '°';
    if (minute) minute += "'";
    if (second) second += '"';

    input.value = [degree, minute, second].join(' ');
  });
})();
<input type="text" />

I do not claim it to be perfect, so I very much appreciate any input.

Unless you know for sure that the degrees, minutes, and seconds are always going to be 2 digits each, this approach will cause issues. I would recommend using 3 separate inputs for each value and automatically tabbing over to the next one when the first one is filled.

If you do know they will always be 2 digits, why not just concat the value with + instead of putting them in an array and joining them?


Make some inputs inside functions instead of main I'm very new to OOP, and I'm trying to do it correctly!
I have been doing a simple friendbook where you can add, remove and show the friends you have. However, I dont know if I should be satisfied or not with my code.

class Friendbook:

    listOfFriends = {}

    def __init__(self, name, lastname):
        self.name = name
        self.lastname = lastname

    def addFriend(self):
        self.listOfFriends[self.name] = self.lastname

    def showFriends(self):
        for firstname, lastname in self.listOfFriends.items():
            print(firstname, lastname)

    def removeFriend(self):
        removeName = input("Enter name to remove\n")
        del self.listOfFriends[removeName]

while True:

    print("1. Add Friend\n2. Showfriends\n3. RemoveFriend\n9. Quit\n")
    try:
        selectedOption = int(input())
        if selectedOption == 1:
            name = input("Enter firstname\n") #Can i move this to a function?
            lastname = input("Enter lastname\n") #Can i move this to a function?
            makeFriends = Friendbook(name, lastname)
            makeFriends.addFriend()
        elif selectedOption == 2:
            makeFriends.showFriends()
        elif selectedOption == 3:
            makeFriends.removeFriend()
        elif selectedOption == 9:
            break
        else:
            print("Not a valid option")
    except:
        print("Only integers allowed")

I'm happy with it except for the code related to selectedoption 1, can I do something to put it inside the function so it looks nicer?
What do you think about my code in general, appreciate feedback!

You are using classes wrong. With your class, you can never have two friend books, because each instance uses the classes mutable listOfFriends. (N.B.: A dictionary called listOf... is not very helpful).

Instead, have one FriendBook object, and let its methods take arguments:

class FriendBook:

    def __init__(self):
        self.friends = set()

    def add(self, firstname, lastname):
        self.friends.add((firstname, lastname))

    def remove(self, firstname, lastname):
        self.friends.remove((firstname, lastname))

    def __str__(self):
        return '\n'.join("{} {}".format(*name) for name in self.friends)

A few notes:

The friends are a set now (this assumes that no two friends share the same first and last name, already a better assumption than yours, which assumed that every friend had a unique last name). You could also make it a list, you would just have to change self.friends.add to self.friends.append.
The magic function __str__ allows simply calling print(fb), where fb is an instance of FriendBook, to print the full list of friends.
I made the names a bit easier. Note that Python has an official style-guide, PEP8, which recommends using PascalCase for classes and snake_case everywhere else.

For your main question, yes you can put these in a function:

def ask_name():
    first_name = input("Enter firstname\n")
    last_name = input("Enter lastname\n")
    return first_name, last_name

I would also slightly rewrite your main function, like this:

if __name__ == "__main__":
    fb = FriendBook()
    while True:
        print("1. Add Friend\n2. Show Friends\n3. Remove Friend\n9. Quit\n")
        try:
            option = int(input())
        except ValueError:
            print("Only integers allowed")
            continue
        if option == 1:
            fb.add(*ask_name())
        elif option == 2:
            print(fb)
        elif option == 3:
            fb.remove(*ask_name())
        elif option == 9:
            break
        else:
            print("Not a valid option")

Notes:

I wrapped it all in a if __name__ == "__main__" guard to allow importing this class from another script.
I use the defined function to ask for the names and use tuple unpacking to pass it to the methods.
I limited the scope of the try...except clause and made it more specific. Your current except covered everything, even, for example, the user pressing Ctrl+C. You should almost always avoid doing this.


Sorting millions of integers Last Friday I was hit with a sorting interview question that I never really had to deal with.

  Develop a your own sorting algorithm.

  It cannot use any other Classes for help.
  It needs to be able to sort an array of millions of integers in size.
  It needs to be as fast as possible.

For instance:

int[] old = {5434, 3454, 2, 0, 356, 896, 7324, 888, 99, 78365, 111};
int highestNumber = 78365;

Would be

int[] new = {0, 2, 99, 111, 356, 888, 896, 3454, 5434, 7324, 78365};

I spent the night trying to come up with my own method to do this. And this is what I came up with.

public class Main {
    public static void main(String[] args) {
        int[] twentyMillion = new int [20000000];
        for (int i = 0; i < a.length; i++) {
            twentyMillion [i] = new Random().nextInt(20000000);
        }
        sortByAccendPro(twentyMillion , 20000000);
    }

    /**
     * Jasz sort algorithim.
     *
     * @param {int[]} twentyMillion - array of twenty million random ints.
     * @param {int} highestNumber - Highest number to sort to.
     */
    public void sortByAccendPro(int[] twentyMillion, int highestNumber ) {
        int[] rangePosition = new int[twentyMillion.length];
        int[] newArray = new int[twentyMillion.length];
        int[] range = new int[highestNumber];
        long time = System.nanoTime();
        for (int i = 0; i < twentyMillion.length; i++) {
            rangePosition[i] = twentyMillion[i];
            range[twentyMillion[i]]++;
        }
        for (int i = range.length - 1, past = twentyMillion.length; i >= 0; i--) {
            range[i] = past - range[i];
            past = range[i];
        }
        for (int i = 0; i < twentyMillion.length; i++) {
            newArray[range[rangePosition[i]]] = twentyMillion[i];
            range[rangePosition[i]]++;
        }
        System.out.println("time = " + (System.nanoTime() - time));
    }
}

Steps:

The first loop has the range of numbers. For instance, if the rangeArray goes from 0 to 3,000,000, it increments every case of each number it finds in that array. So every time it finds 2,750,000 it increments that position in the rangeArray.
The second loop works backwards from the max position in the rangeArray. So if the size is 3,000,000 and it has 100,000 cases of 3,000,000 it says that 3,000,000 will start at 2,900,000 and go to the max.
The 3rd loop loops back through the main array grabbing the same index in the range array and plugging the number in the correct position in the newArray.

It handles duplicates and with a little tinkering you can make it sort many other things. It uses more memory than I wanted in order to do sorting but wow its lightening fast. I never thought of looking into how these sorting algorithms work until I did this but can find nothing to compare it to.

What algorithm does this resemble and what can I do to make it better?

The algorithm you have implemented is known as counting sort.  Its run-time cost is linear in the size of the input – faster than any comparison-based sorting algorithm can possibly get.  (At the cost of being also linear in the difference of the maximum and minimum element in the input.)  Congratulations if you've come up with this idea on your own.  Since they already give you the largest number in the array as additional input, it seems very likely that they wanted to see this algorithm.  (Of course, you can find the maximum yourself in linear time, if needed.)

Remarks about your code:

The rangePosition array is initialized with an exact copy of twentyMillion and then only ever read.  Why did you create it instead of using twentyMillion directly?
If twentyMillion contains a negative number, your implementation will explode.  Maybe you simply forgot to mention that all the inputs are guaranteed to be non-negative?  Otherwise, you'd also need to know the minimum value and normalize your keys to that.  (This could also help you save something if the minimum is much larger than zero.)
If the highestNumber is extremely large, you will get a problem.  For example, you will probably not be able to allocate a new int[Integer.MAX_VALUE] without receiving an OutOfMemoryError.  (And if you allow for negative numbers in the input, you might even need an array larger than Integer.MAX_VALUE!)  And even if you could allocate it, iterating over it will take forever.  If you want to make your code more robust, you could decide by some heuristic whether the combination of twentyMillion.length and highestNumber warrants the overhead of counting sort or you'd be better off using a comparison-based O(n log(n)) fallback-algorithm.
twentyMillion is a poor name for a variable that does not necessarily name an array of length 20M.


Listing last 3 items (or all if less than 3) in an ArrayList Writing a code to return the last 3 items in a list, if the list has 3 or more elements, otherwise return all list items.

But the code looks really bad.
What is a better way to write the same code?

   public void test_arrayListItems() {
        ArrayList<String> arrayList = new ArrayList<>();
        arrayList.add("item1");
        arrayList.add("item2");
        arrayList.add("item3");
        arrayList.add("item4");
        arrayList.add("item5");
        arrayList.add("item6");
        arrayList.add("item7");
        arrayList.add("item8");

        String items = "";
        int j = 0;
        int k = 0;

        if (arrayList.size() <= 2) {
            j = arrayList.size() - 1;
        } else if (arrayList.size() > 2) {
            j = arrayList.size() - 1;
            k = arrayList.size() - 3;
        }
        for (int i = k; i <= j; i++) {
            items = items + "- " + arrayList.get(i) + " ";
        }
        logger.info(items);
    }

thanks

Is a best practice to define a variable as type of the interface the instance implement it(see OOP principles)
Just a little bit out of scope, you can declare your arrayList inline, like in the example I wrote.
Try to use available features of List(in this example) like subList rather than cycle(what if you have thousands of elements?)
toString on a list will return a string with those list, you don't have to concatenate them
it is a bad practice to concatenate strings in cycles because create new strings every time you add + " "

See an alternative solution below:

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class MainTest {
    public static void main(String[] args) {
        final int wantedSize = 3;
        final List<String> arrayList = new ArrayList<>(Arrays.asList("item1", "item2", "item3", "item4"));
        final int listSize = arrayList.size();
        final List<String> result = (listSize >= wantedSize) ? arrayList.subList(listSize-wantedSize, listSize): arrayList;
        System.out.println(result.toString());
    }
}


Length units converter I am creating a very small application to demonstrate solid principles and also a brief implementation of a builder pattern, does anyone have any feedback as to how this could be improved or how it breaks the solid principles? The app is a small app that just converts units, e.g. yards to meters, inches to cms.

Interface

public interface IConverter
{
    double ConversionRate { get; set; }
    string[] ConvertedUnits { get; set; }
    string DisplayText { get; set; }

    string[] Convert(string input);
}

Class implementing interface

 public class Converter : IConverter
    {
        public double ConversionRate { get; set; }
        public string[] ConvertedUnits { get; set; }
        public string DisplayText { get; set; }

        public Converter(double conversionRate, string[] convertedUnits, string displayText)
        {
            ConversionRate = conversionRate;
            ConvertedUnits = convertedUnits;
            DisplayText = displayText;
        }

        public string[] Convert(string input)
        {
            if (!input.Contains('\n'))
            {
                double d = double.Parse(input, CultureInfo.InvariantCulture) * ConversionRate;

                string[] array = new string[1];
                array[0] = d.ToString();

                return array;
            }
            else
            {
                double[] doubles = new double[input.Split('\n').Count()];

                for (int i = 0; i < input.Split('\n').Count(); i++)
                {
                    double value = double.Parse(input.Split('\n')[i]);
                    doubles[i] = value;

                    string.Format("{0}", value * ConversionRate);
                }

                string[] strings = new string[doubles.Length];

                for (int i = 0; i < input.Split('\n').Length; i++)
                {
                    strings[i] = string.Format("{0}", doubles[i] * ConversionRate);
                }

                return strings;
            }
        }
    }

Builder (abstract class)

public abstract class ConverterBuilder
    {
        protected double _conversionRate;
        protected string[] _convertedUnits;
        protected string _displayText;

        public ConverterBuilder AddConversionRate(double conversionRate)
        {
            _conversionRate = conversionRate;
            return this;
        }

        public ConverterBuilder AddConversionRate(string[] convertedUnits)
        {
            _convertedUnits = convertedUnits;
            return this;
        }

        public ConverterBuilder AddDisplayText(string displayText)
        {
            _displayText = displayText;
            return this;
        }

        public Converter Build()
        {
            return new Converter(_conversionRate, _convertedUnits, _displayText);
        }

    }

Builder implementing abstract builder class

public class UnitConverterBuilder : ConverterBuilder
{
    public UnitConverterBuilder()
    {

    }
}

Controller

public IActionResult Convert(string text, double conversionRate)
{
    Converter converter = new UnitConverterBuilder()
    .AddConversionRate(conversionRate)
    .Build();

    string output = "";
    for (int i = 0; i < converter.Convert(text).Count(); i++)
    {
        output += converter.Convert(text)[i] + Environment.NewLine;
    }

    return Content(output);

}

View

    Enter values to convert

<form asp-controller="Home" asp-action="Convert" method="post">
    <textarea id="text" name="text" rows="10" cols="40"></textarea>
    <select name="conversionRate">
        <option value="">Please Select</option>
        <option value="0.9144">Yards to Meters</option>
        <option value="2.54">Inches To Centimeters</option>
    </select>
    <button>Convert</button>
</form>

The app is built using .net core, any feedback greatly appreciated :)

Only targeting public string[] Convert(string input)

Because this is a public method you should validate the input. Right now if one passes null this will throw an ArgumentNullException which isn't that bad but the stacktrace belongs to System.Linq.Enumerable.Contains() which you should hide.
Just testing against null or using string.IsNullOrWhiteSpace()` will do the trick.
You are calling input.Split('\n') many many times. It would be much better to call it once and store the result in a variable.
Don't use Count() method on an array, Using Count() use the Length property instead.
You should use the overloaded Split() method which takes a [StringSplitOptions][2] enum like StringSplitOptions.RemoveEmptyEntries.
Instead of using double.Parse() you should consider to use double.TryParse() which won't throw an exception if the current string doesn't represent a double like e.g a letter.
This string.Format("{0}", value * ConversionRate); can be safely removed because the result isn't assigned to anything.
Instead of returning a string[] you should consider to use an IEnumerable<string or better just an IEnumerable<double> which is more straightforward. Sure that means you need to change your interface as well.
Using a foreach will make the code shorter because you won't need to check for \n.

Implementing the mentioned points will lead to

string version

public IEnumerable<string> Convert(string input)
{
    if (string.IsNullOrWhiteSpace(input))
    {
        yield break;
    }
    var values = input.Split(new char[] { '\n' }, StringSplitOptions.RemoveEmptyEntries);
    foreach (var value in values)
    {
        double current;
        if (double.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out current))
        {
            yield return (current * ConversionRate).ToString();
        }
    }
}

double version

    public IEnumerable<double> Convert(string input)
    {
        if (string.IsNullOrWhiteSpace(input))
        {
            yield break;
        }
        var values = input.Split(new char[] { '\n' }, StringSplitOptions.RemoveEmptyEntries);
        foreach (var value in values)
        {
            double current;
            if (double.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out current))
            {
                yield return (current * ConversionRate);
            }
        }
    }

Ok, I need to target Controller.Convert() as well

This is really really bad:

string output = "";
for (int i = 0; i < converter.Convert(text).Count(); i++)
{
    output += converter.Convert(text)[i] + Environment.NewLine;
}

return Content(output);

Assume you have text = "\n1\n2\n3\n4\n5.......\n1000" then your code will once call converter.Convert() at
for (int i = 0; i < converter.Convert(text).Count(); i++)
which results in Count() = 1000 hence the loop will be executed 1000 times which calls converter.Convert(text) 1000 times.
In addition using output += on strings will lead to very bad performance because each time a new string object is created because strings are immutable.
Better use a StringBuilder.

Assuming you use the IEnumerable<double> Convert() this will lead to

    StringBuilder sb = new StringBuilder(1024);
    foreach (var value in converter.Convert(text))
    {
        sb.AppendLine(value.ToString());
    }

    return Content(sb.ToString());


Generating Even Random Numbers I have this class which is used as part of a game. It needs to generate Random Even values, which is done by generating random numbers until the result is even.

Is there a better way to do this?

Also, I currently have the methods implemented as private instance methods, but should I declare generateRandomNumber() and evenNumber(number) as static methods? Will it have any benefits?

public class Game {
    //...

    public void opponentSaysEvenNumber() {
        int number = generateRandomEvenNumber();
        System.out.println("Opponent: " + number);
    }

    private int generateRandomEvenNumber() {
        Random random = new Random();
        int number = random.nextInt();
        while (!evenNumber(number)) {
            number = random.nextInt();
        }
        return number;
    }

    private boolean evenNumber(int number) {
        return (number % 2) == 0;
    }
}

Yes.  I would go one step further and declare your functions as private static final, if possible.  The combination of those three keywords means that the code would be unaffected by any instance variable, any superclass, or any subclass, and is also not callable by any code external to the class.  Therefore, the compiler has enough of a hint that it could decide to inline the entire function.

I would also rename evenNumber(int number) to isEven(int number).  There is a convention in Java that functions named isSomething() return a boolean and have no side effects.  Your function meets those criteria.

To generate a random even number, you could just take random.nextInt() & -2 to mask off the least significant digit.  That would be more efficient than looping, testing, and discarding.  In that case, the whole question about helper functions would be irrelevant.

It's bad practice to create a new instance of Random every time you want to generate one random number, though.  The pseudorandom number generator actually carries some state, even if you don't think of it that way.  You should therefore use a private static variable to store the Random object.


Separating integers substrings from those with alphabetical letters The goal of my code is given a string, to separate the integer substrings from those containing solely alphabetical letters, with the individual substrings in a list.

For example:

Input: 1h15min
Output: ["1", "h", "15", "min"]

Here is the code:

def separate_text(input_text):
    if not input_text:
        return None
    character_list = []
    current_substring = ""
    string_type = ""
    for character in input_text:
        if character in "0123456789":
            if string_type in ("Int", ""):
                current_substring += character
                if string_type == "":
                    string_type = "Int"
            else:
                character_list.append(current_substring)
                string_type = "Int"
                current_substring = character
        elif character not in "0123456789":
            if string_type in ("Str", ""):
                current_substring += character
                if string_type == "":
                    string_type = "Str"
            else:
                character_list.append(current_substring)
                string_type = "Str"
                current_substring = character
    character_list.append(current_substring)
    return character_list

The function functions as follows:

Take an input string (if empty, return None)
Begin looping through the string, character by character
If the character is an integer

If string_type is Int or "", append the character to current_substring and change string_type if needed
If string_type is Str, append the substring to the character_list, reassign current_substring to whatever the character is, and change the string_type to "Int".

If the character is an string

If string_type is Str or "", append the character to current_substring and change string_type if needed
If string_type is Int, append the substring to the character_list, reassign current_substring to whatever the character is, and change the string_type to "Str".

End of the whole string? Append current_substring to the list and return the character_list.

My questions here are:

Is there any way to make the program for Pythonic?
Can I possible shorten my code? The two main if statements in the for loop have nearly identical code.

Note that I do not count floats so 1.15min returns ['1', '.', '15', 'min'].

The elif in

   if character in "0123456789":
        // ...
    elif character not in "0123456789":
        // ...

can be replaced by an else.

if string_type in ("Int", ""):
    current_substring += character
    if string_type == "":
        string_type = "Int"
else:
    character_list.append(current_substring)
    string_type = "Int"
    current_substring = character

can be simplified to

if character in "0123456789":
    if string_type == "Str":
        character_list.append(current_substring)
        current_substring = ""
    string_type = "Int"
    current_substring += character

and similarly for the non-digits case. However, using strings
("", "Str", "Int") for the current state is error-prone.
An enumeration would be an alternative, or in this case simply a boolean:

in_digits = None
for character in input_text:
    if character in "0123456789":
        if in_digits is False:
            character_list.append(current_substring)
            current_substring = ""
        in_digits = True
        current_substring += character
    else:
    // ...

And now the two cases can be combined into one:

in_digits = None
for character in input_text:
    is_digit = character in "0123456789"
    if in_digits == (not is_digit):
        character_list.append(current_substring)
        current_substring = ""
    in_digits = is_digit
    current_substring += character

More remarks:

if not input_text:
    return None

When called with an empty string, I would expect the function to return
an empty array, not None.

character_list = []

The variable name is slightly misleading, this is a list of strings.

Summarizing the suggestions so far, we have

def separate_text(input_text):
    string_list = []
    current_substring = ""
    in_digits = None
    for character in input_text:
        is_digit = character in "0123456789"
        if in_digits == (not is_digit):
            string_list.append(current_substring)
            current_substring = ""
        in_digits = is_digit
        current_substring += character
    string_list.append(current_substring)
    return string_list

Finally note that the same can be simply achieved by splitting the
string on a regular expression:

import re

def separate_text(input_text):
    return [item for item in re.split('(\d+)', input_text) if item]


Count to target with JavaScript I'm looking for (and be as brutal as you like) ways to improve the code or the algorithm (I'm aware there should be comments) - I'm a recreational programmer and would like to be improving my skills. My next step is to get the targetNumber value from a nearby file without losing the current count.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN "http://www.w3.org/TR/html4/strict.dtd">

  <title>Count to a particular target</title>

Starting

function count(from, to, targetElem) {
    targetElem.innerHTML = 'Total wordcount: ' + from;

    if (from == to) return;

    from < to ? from++ : from--;

    var changeTime = Math.max(20, 1000 - Math.abs(from - to)) / 2;

    setTimeout(function() {count(from, to, target);}, changeTime);
}

count(50, 0, document.getElementById("t"));

A few notes:

avoid global variables to conserve state - use function arguments instead
always use var to declare variables. Otherwise they are in the global scope and you don't want this
added error checking (i.e. whether values are numeric and target is valid) might not be a bad idea, I left it out for clarity here
return early on known conditions, this reduces complexity in the function body
the ternary operator condition ? ifTrue : ifFalse can be used in other ways than assigning a value


Preg_replace improvement I would like to improve my preg_replace regex.
This is to clean a features list.

I want allow for the begining of each line:

alphanumeric characters
== and alphanumeric characters
-- alphanumeric characters
++ alphanumeric characters
** alphanumeric characters

my regex:

$features = "
    == Category one
    → feature 1
    ++ feature 2
    -- feature 3
    ==-
    ! feature 4
    % feature 5";

preg_replace('/' .
          '^[^[:alnum:]]{0,}(==-.*)$' . '|' .              // ==-
          '^[^[:alnum:]]{0,}([\+|\-|\*|\=]{2}.*)$' . '|' . // ==
          '^[^[:alnum:]]{0,}(.*)$' .                       // abc
          '/mu', '$1$2$3',
          $features);

Dirty features:

== Category one
→ feature 1
++ feature 2
-- feature 3
==-
! feature 4
% feature 5

Clean:

== Category one
feature 1
++ feature 2
-- feature 3
==-
feature 4
feature 5

It works, but i think there is a better way ...
Thanks

Maybe this one : #^[^-=+*\w]{2}\s*#um (that is ~500% faster than yours):

<?php
$list = '
== Category one
→ feature 1
++ feature 2
-- feature 3
==-
! feature 4
% feature 5
** feature 6
feature 7';

$list = preg_replace('#^[^-=+*\w]{2}\s*#um', '', $list);
print_r($list);

Whose output is:

== Category one
feature 1
++ feature 2
-- feature 3
==-
feature 4
feature 5
** feature 6
feature 7

Subsidiary question

You can complete the regexp #^[^-=+*\w]{2}\s*#um by the regexp
#^([-=+*])(?!\1|\s)#um to refuse +|-|=|* without whitespace after.

It is possible to combine the two regexps but at the expense of performance.

<?php
function printResult($label, $list, $start, $end) {
  printf(
    "%s: %F s\n%s\n\n",
    $label,
    $end - $start,
    print_r($list, true)
  );
}

$list = '== Category one
→ feature 1
++ feature 2
-- feature 3
==-
! feature 4
% feature 5
** feature 6
feature 7
-line1
-- line2';

// Case 1: regrouping the regexps
$time_start = microtime(true);
$list = preg_replace('#^[^-=+*\w]{2}\s*|^([-=+*])(?!\1|\s)#um', '', $list);
$time_end = microtime(true);
printResult('With one regexp', $list, $time_start, $time_end);

// Case 2: a faster way
$time_start = microtime(true);
$list = preg_replace('#^[^-=+*\w]{2}\s*#um', '', $list);
$list = preg_replace('#^([-=+*])(?!\1|\s)#um', '', $list);
$time_end = microtime(true);
printResult('In two pass, a faster way', $list, $time_start, $time_end);

Whose output is:

With one regexp: 0.000151 s
== Category one
feature 1
++ feature 2
-- feature 3
==-
feature 4
feature 5
** feature 6
feature 7
line1
-- line2

In two pass, a faster way: 0.000024 s
== Category one
feature 1
++ feature 2
-- feature 3
==-
feature 4
feature 5
** feature 6
feature 7
line1
-- line2

Note: preg_replace replaces the overall catch, therefore it is useless to try to capture the full line.


Model Association (Rails 5 API) I have a Character model that has a CharClass and ClassPerks child.

Currently I am duplicating my logic a lot by manually assigning data.

My Character index and create:

def index

    @campaign = Campaign.find_by_id(params[:campaign_id])
    @characters = @campaign.characters
    render json: {
        characters: @characters
    }
end

def create
    @campaign = Campaign.find_by_id(params[:campaign_id])
    @charClass = CharClass.find_by_id(params[:character_char_class_id])
    @character = @campaign.characters.new(character_params)
    @character.char_class_id = params[:character_char_class_id]

    if @character.save!
        render status: 201, json: {

            message: "Successfull added this character to the campaign!",
            character_name: @character.character_name + ' the ' + @charClass.class_name,
            character_class: @charClass.class_name,
            character_level: @character.character_level,
            character_experience: @character.character_experience,
            character_gold: @character.character_gold,
            character_perks: get_names(@character.character_perks),
            character_image: @character.character_image

        }
    else
        render status: 404, json: {
            message: "Something went wrong: Check line 24 of the Character Controller"
        }
    end
end

I am manually assigning character_class and character_image despite both of these ALWAYS being the same based on their character_class.

Inside of the character.rb I am doing this to assign them post-create:

    after_create :add_perks, :save_class

    def add_perks
        self.class_perks.each do |perk|
            self.character_perks.create(class_perk_id: perk.id)
        end
    end

    def save_class()
        case self.char_class_id
            when 1 # Mindthief
                self.character_image = "https://gloomhavenil.files.wordpress.com/2017/12/11849117_616974988445216_217288420_n.jpg?w=480"
                self.character_class = "Mindthief"
                self.save
            when 2 # Tinkerer
                self.character_image = "http://www.cephalofair.com/wp-content/uploads/2015/04/Quatryl-Tinkerer.jpg"
                self.character_class = "Tinkerer"
                self.save
            # when 'Sawbones'
            # else
        end
    end
end

It is working, but I am unsure how to do this better/appropriately. I removed useless fields but you'll see below I have the relations set up appropriately.

  create_table "char_classes", force: :cascade do |t|
    t.string "class_name"
    t.string "class_image"
    t.string "class_perks"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
  end

 create_table "characters", force: :cascade do |t|
    t.string "character_class"
    t.string "character_image"
    t.integer "char_class_id"
    t.index ["campaign_id"], name: "index_characters_on_campaign_id"
    t.index ["char_class_id"], name: "index_characters_on_char_class_id"
  end

I can do Character.first.char_class.class_name and I will get the appropriate result; so I am back to assuming this method I'm following is very incorrect.

You need to use ActiveModelSerializers (AMS) to serialize your json responses based upon the instances of your class.

class CampaignsController < ApplicationController
  def index
    render json: { characters: campaign.characters }
  end

  def create
    @character = campaign.characters.new(character_params.merge(char_class_id: char_class.id))

    if @character.save!
        render json: @character, serializer: CharacterSerializer, status: 201 # This should automatically be formatted by AMS
    else
        not_found('Something went wrong: Check line 24 of the Character Controller')
    end
  end

  private

  def campaign
    @campaign ||= Campaign.find_by_id(params[:campaign_id])
  end

  def char_class
    @charClass ||= CharClass.find_by_id(params[:character_char_class_id])
  end
end

This is how you AMS class for Character class should look like

class CharacterSerializer < ActiveModel::Serializer
  attributes :message, :character_name, :character_class, :character_level, :character_experience, :character_gold, :character_perks, :character_image

  def message
    'Successfull added this character to the campaign!'
  end

  def character_name
    return object.character_name + ' the ' + character_class # check the associations
  end

  def character_class
    object.charClass.class_name
  end

  def character_perks
    # Unsure what this get names methods does so im leaving it for you to refactor
    get_names(object.character_perks),
  end

end

Also add this to your ApplicationController

class ApplicationController < ActionController::Base

  # Add this to your application controller

  def not_found(message = nil)
    render json: { message }, status: 404
  end

end

I'll update the refactored model class based upon your info soon. Hope this helps for now.


Removing duplicates from a linked list without a buffer I am practicing a problem in "Cracking the Coding Interview". The problem is to remove duplicates in a linked list without the use of a buffer. I interpreted this as not using a list or any large data structure such as a hash to store unique nodes.

My algorithm is inefficient I think. It iterates an anchor across the linked list. From that anchor, a second sub-iteration occurs which removes any nodes that is the same as the anchor. Thus, there are a total of two loops. I think the time complexity is either O(n^2) or O(nlogn)

Any better algorithms are welcome. Since I am a novice in Python, please give me suggestions on my coding style.

class Node(object):

    def __init__(self, data):
        self.data = data
        self.next = None

    def getData(self):
        return self.data

    def setNext(self, node):
        self.next = node

    def getNext(self):
        return self.next

class LinkedList(object):

    def __init__(self, dataList):
        assert len(dataList) > 0
        self.head = Node(dataList[0])
        iterNode = self.head

        for i in range(1, len(dataList)):
            iterNode.setNext(Node(dataList[i]))
            iterNode = iterNode.getNext()
        iterNode.setNext(None)

    def printList(self):
        iterNode = self.head

        while iterNode is not None:
            print(iterNode.getData())
            iterNode = iterNode.getNext()

    def removeDuplicates(self):
        assert self.head is not None
        anchor = self.head

        while anchor is not None:
            iterator = anchor
            while iterator is not None:
                prev = iterator
                iterator = iterator.getNext()
                if iterator is None:
                    break
                if iterator.getData() == anchor.getData():
                    next = iterator.getNext()
                    prev.setNext(next)

            anchor = anchor.getNext()

dataList = ["hello", "world", "people", "hello", "hi", "hi"]

linkedList = LinkedList(dataList)
linkedList.printList()
linkedList.removeDuplicates()
print("\n")
linkedList.printList()

Output:

hello
world
people
hello
hi
hi

hello
world
people
hi

Problem statement

The problem is underdefined. In a real-life interview I'd ask immediately, do we care about the stability? If yes, your solution is good. If no, sort the list first.
Complexity

\$O(n^2)\$ and \$O(n\log n)\$ are quite different beasts. This algorithm is \$O(n^2)\$ obviously. If sorting is allowed, there is a \$O(n \log n)\$ solution.
getters and setters

Python doesn't have a concept of public and private. At the end of the day everything is public. getData, getNext and setNext do not serve any purpose but generating entropy.

Even in the language which encourages such approach it only makes sense when there is an invariant to maintain. Unrestricted getter and setter are as good as a public member.