Run time complexity of printing a binary tree level by level I have some doubts about running time of following question. The problem is printing a binary tree level by level. For example if tree is like

            5
           / \
          2   6
         / \   \
        7   1   8

Then the output should be like

5
2 6
7 1 8

TreeNode structure is very simple and looks like below;

public class TreeNode {

    public TreeNode left;
    public TreeNode right;
    public int      value;

    public TreeNode(int value) {
        this.value = value;
    }
}

And the code is shown below:

public void printByLevel(TreeNode root) {

    if (root == null) {
        throw new IllegalArgumentException();
    }

    Queue<TreeNode> current = new LinkedList<>();
    current.add(root);

    while (!current.isEmpty()) {

        Queue<TreeNode> parents = current;
        current = new LinkedList<>();

        // print parents
        for (TreeNode parent : parents) {
            System.out.print(parent.value + "  ");
        }

        System.out.println();

        // construct next level by adding children of each parent
        for (TreeNode parent : parents) {

            if (parent.left != null) {
                current.add(parent.left);
            }

            if (parent.right != null) {
                current.add(parent.right);
            }
        }
    }
}

Now can I say running time of this algorithm is \$O(N)\$ because every is node processed only once? Or should I say it is \$O(m*N)\$, where \$m\$ refers to number of nodes at the current level because I am printing nodes at each level?

Your algorithm is plain \$O(n)\$. If you double the number of nodes, your runtime doubles. If you double the number of nodes at the current level, you also double the total number of nodes (or are you planning an unbalanced tree?).

Your algorithm is OK, but there are some deviations from the classics...

The use of a queue here is not quite right, or, rather, the way you are using it is not taking advantage of its queue features. In fact, you are using it as a List, and not a Queue. A Queue should be using the methods add, remove and element (or offer, poll, and peek). You are using the iterate method though instead.

Also, instead of emptying the queue, you create a new one, and throw the old one out. This is inefficient.

A more traditional implementation of the algorithm will use a single queue, and add a 'marker' in the queue to indicate the end of a 'row' in the tree.

Consider using 'null' as a marker, to show you have reached the end of the row, as follows:

queue.add(root);
queue.add(null);
while (!queue.isEmpty()) {
    Node n = queue.remove();
    if (n == null) {
        // end of a row
        System.out.println();
        // which also means we have no more kids to add, so
        // if there is more data, we mark the next end of the *next* row:
        if (!queue.isEmpty()) {
            queue.add(null);
        }
    } else {
        System.out.print(n.value + "  ");
        if (n.left != null) {
            queue.add(n.left);
        }
        if (n.right != null) {
            queue.add(n.right);
        }
    }
}

Sometimes people use a special marker instance instead of null, perhaps something like:

private static final ENDOFROW = new Node(-1);

Then, instead of the null check, you can do:

if (n == ENDOFROW) {

and you can 'add' an ENDOFROW instance as well instead of the null.


Storing function metadata In a project, we have a file that contains something like a script which we need to parse. At the moment the validation is tough since we have an array of functions returing variable and taking variableList (basically std::variant<>and vector of variants, see definition below)
I am trying to have function metadata available (number of input arguments and its type, possibly return value type for validating the input "script". I would like to replace std::function with something like this:

using variable = std::variant<bool, int, double, std::string>;
using variableList = std::vector<variable>;

enum {
    BOOL_TYPE = 0,
    INT_TYPE = 1,
    DOUBLE_TYPE = 2,
    STRING_TYPE = 3,

    VARIABLE_BOOL_TYPE = 0 | 0x80,
    VARIABLE_INT_TYPE = 1 | 0x80,
    VARIABLE_DOUBLE_TYPE = 2 | 0x80,
    VARIABLE_STRING_TYPE = 3 | 0x80
};

struct Function
{
    std::string                                 fname;
    std::vector<int>                            indexes;
    std::function<variable(variableList &&)>    fn;

    variable operator()(variableList && list)
    {
        if (indexes.size() != list.size() &&
            (indexes.size() == 0 || (indexes[0] & 0x80) != 0x80))
        {
            std::cout << "Invalid number of arguments" << std::endl;
            return variable(0);
        }

        int idx = 0;
        for (auto i : list)
        {
            if ((indexes[0] & 0x80) ? i.index() != (indexes[0] & 0x03), idx++ : i.index() != indexes[idx++])
            {
                std::cout << "Invalid argument type" << std::endl;
                return variable(0);
            }
        }

        return fn(std::forward<decltype(list)>(list));
    }
};

int main()
{
    Function f = { "fName", {BOOL_TYPE, INT_TYPE, BOOL_TYPE}, [](variableList && list) -> variable {
        for (auto i : list)
        {
            std::visit([](auto && val) -> void {
                std::cout << val << std::endl;
            }, i);
        }
        return variable(0);
    }};

    auto arguments = {variable(true),variable(true),variable(true)};
    auto returnValue = f(arguments);
}

The project supports c++17. Any modifications, suggestions are welcome.

EDIT: Initially I wanted to use templates instead of indices and enum, but I wasn't quite successful with creating array of Function class with different template arguments.

Prefer enum class with a name, and probably rename the enums, e.g. BOOL_TYPE becomes TYPE::BOOL or however you'd like to do the naming. Why do you want them to be an unnamed enum type?

You're printing an error to cout when there's an error. At the very least, print to cerr, so that someone running this might be able to filter output if they'd like to. I would personally either return a variant<variable, error_message_type> or throw an exception if there's an error like that though. Otherwise errors could easily slip through unnoticed.

This validation loop is inefficient and confusing.

Prefer extracting the conditional if possible. It appears to be ensuring that all the types are the same if it is a "VARIABLE_" type. Perhaps "variadic" may be more appropriate?

    int idx = 0;
    for (auto i : list)
    {
        if ((indexes[0] & 0x80) ? i.index() != (indexes[0] & 0x03), idx++ : i.index() != indexes[idx++])
        {
            std::cout << "Invalid argument type" << std::endl;
            return variable(0);
        }
    }

indexes could take a type enum instead of int if it had a name. Why is it called indexes? You may be able to leverage more of std::variant and possibly not need the enum at all.

for (auto i : list) You probably want auto & i, since this will result in string copies in the case of string variants.

return fn(std::forward<decltype(list)>(list));
This is unnecessary. std::forward is useful for templated types, but you know the type of list, it's a variableList &&.

You could move the list, but I'm not sure that's what you really want.


Refactor a legacy application to a less-unwieldy state Follow-up posted here: Refactor this method to a cleaner state (continued)

I am refactoring a legacy application. I DO NOT have the ability to change the Permissions, CustomerReport or the ListItems classes. They are in external dlls that are given to me. I realize that the ref usage is dumb and it makes NO SENSE. I am going to work with my peers to follow better practices.

This whole class looks ugly and mind you I have refactored some of it out into methods but it still is too large and unwieldy:

public class ReportService : IReportService
{
    private readonly Permissions _Permissions;

    public ReportService()
    {
        _Permissions = new Permissions();
    }

    public IList<Node> GetFor(string userGroup)
    {
        var nodes = new List<Node>();
        var ListItems = new List<ListItem>();

        //Yes this is silly since statusReturn is never used
        var statusReturn = _Permissions.GetReportsForUserGroupName(userGroup, ref ListItems);

        if (ListItems.Any())
        {
            var initialReportType = ListItems[0].value;
            var initialCrs = ListItems[0].key;
            var parent = AssembleNode(initialReportType);
            var child = AssembleNode(initialCrs, parent);
            foreach (var item in ListItems)
            {
                var customerReport = (CustomerReport)item.dataObject;
                var grandChild = AssembleGrandChild(child, customerReport);
                if (item.value.Equals(initialReportType))
                    if (item.key.Equals(initialCrs))
                        child.Children.Add(grandChild);
                    else
                    {
                        initialCrs = item.key;
                        parent.Children.Add(child);
                        child = AssembleNode(item.key, parent);
                        child.Children.Add(grandChild);
                    }
                else
                {
                    initialReportType = item.value;
                    initialCrs = item.key;
                    parent.Children.Add(child);
                    nodes.Add(parent);
                    parent = AssembleNode(item.value);
                    child = AssembleNode(item.key, parent);
                    grandChild = AssembleGrandChild(child, customerReport);
                    child.Children.Add(grandChild);
                }
            }
            parent.Children.Add(child);
            nodes.Add(parent);
        }
        return nodes;
    }

    private static Node AssembleNode(string value, Node parent = null)
    {
        //This is an extension method on object that was used at my last dev shop.
        //I got used to using it so out of habit it is here
        return parent.IsNotNull() ?
            new Node(parent){ Text = value, Value = value } :
            new Node { Text = value, Value = value };
    }

    private static Node AssembleGrandChild(Node child, CustomerReport customerReport)
    {
        return new Node(child)
        {
            Text = customerReport.FileName,
            Value = customerReport.ReportKey.ToString(),
            Title = customerReport.Description
        };
    }
}

I really want to refactor this to a better state; any suggestions?

I would put:

var ListItems = new List<ListItem>();
//Yes this is silly since statusReturn is never used
var statusReturn = _Permissions.GetReportsForUserGroupName(userGroup, ref ListItems);

in a method that just returns the list. So you are isolating the madness a bit.

I'm not a C# programmer, but wouldn't listItems be better than ListItems?

I honestly did not understand what the code is doing.  Maybe you could explain it a little bit in your post.

It seems that you have a tree structure in there.  So why transform a list which originally seems to contain a tree to another list, which should also be a tree?


Is there any way to simplify this code I'd like to simplify  this Scala code:

private def getToken() = {
    val token = getTokenFromDb()
    token match {
      case Some(x) => token
      case None => {
        val remoteToken = getTokenRemote()
        remoteToken match {
          case Some(x) => {
            writeTokenToDb(x)
            remoteToken
          }
          case None => None
        }
      }
    }
  }

private def getToken(): Option[Token] = {
  def remoteToken =
    getTokenRemote() map { r => writeTokenToDb(r); r }
  getTokenFromDb orElse remoteToken
}

Please write always the type of a function for outer functions - it makes it far more readable to understand the source code (often it is useful to add the types to inner functions/declarations as well).

Beside from that you can also write case t: Some => t or case t @ Some(_) => t to express that you return the matched type. Your way of case Some(x) => token is hard to understand, because one needs to know what's token.

Furthermore, you don't need braces around a match block, you can simply write

case x =>
  a
  b

And naming a variable in a similar way as a function, like in val token = getToken() is completely unnecessary, when it is only used once, it also doesn't increase readability. In such cases just inline the function call: getToken match { ... }.


Creating Extension Method to map entity with subentities object to Poco object I am trying to create an extension method that builds a POCO object (copies all the fields) for an Entity Object.

When Entity object is simple (no navigation, no sub collections), it works fine.
I want to improve it so it could also deal with sub-entities.

For this example I take Nortwind database and use Customer and Order entities.
My POCOs:

 public class CustomerPOCO : BaseDataItemModel
    {
        public string CustomerID { get; set; }
        public string CompanyName { get; set; }
        public string ContactName { get; set; }

        List<OrderPOCO> orders;

       [PropertySubCollection]
        public List<OrderPOCO> Orders
        {
            get { return orders; }
            set { orders = value; }
        }

        public CustomerPOCO()
        {
            orders = new List<OrderPOCO>();
        }

    }

public class OrderPOCO : BaseDataItemModel
{

   public int OrderID { get; set; }
   public string ShipName { get; set; }
   public string ShipCity { get; set; }

}

Extension Method:

 public static void CopyPropertiesFrom<TObjectTarget>(
           this ICollection<TObjectTarget> targetitems, System.Collections.IEnumerable sourceitems)
            where TObjectTarget : new()
        {
            foreach (var source in sourceitems)
            {

                TObjectTarget targetObject = new TObjectTarget();

                PropertyInfo[] allProporties = source.GetType().GetProperties();

                PropertyInfo targetProperty;
                foreach (PropertyInfo fromProp in allProporties)
                {
                    targetProperty = targetObject.GetType().GetProperty(fromProp.Name);

                    if (targetProperty == null) continue;
                    if (!targetProperty.CanWrite) continue;

                    //check if property in target class marked with SkipProperty Attribute
                    if (targetProperty.GetCustomAttributes(typeof(SkipPropertyAttribute), true).Length != 0) continue;

                    //Sub Collection -> set it here
                    if (targetProperty.GetCustomAttributes(typeof(PropertySubCollection), true).Length != 0)
                    {

                    }
                    else

                        targetProperty.SetValue(targetObject, fromProp.GetValue(source, null), null);

                }

                targetitems.Add(targetObject);

            }
        }

static void Main(string[] args)
 {
      List<CustomerPOCO> customers;
      using (NorthwindEntities northcontext = new NorthwindEntities())
      {
          var cus = from customer in northcontext.Customers
                  select customer;

          customers = new List<CustomerPOCO>();
          customers.CopyPropertiesFrom(cus);

      }
   }

Note: Extension should know the type of sub-entity and a type of sub-poco...

For this task, i would use AutoMapper, it does exactly what you need, and can work with complex entities.

If you are writing this mapper yourself, consider using expression compilation feature.
Such code will execute at the speed of a custom written code, with all reflection interpretation done beforehand. See the code example for Expression.MemberInit

As for the sub-objects in your code, you will figure out the recursive procedure, as soon as you rewrite your extension to work with a Type argument, not a generic argument. This way you can easily call the procedure for the properties, knowing their type from PropertyInfo.


Generate XKCD passwords from single Bash line There are a few "XKCD password" generators out there, but the task seems so straight forward, so I wanted a compact and simple solution. My requirements were:

One-line alias (or few-line function) to put in .profile
Standard Unix commands (require no packages)
Column output (like pwgen)
Titlecase words (for readability)
True randomness

(I'm happy to renounce command line options in exchange for simplicity.)

for i in {1..32}; do grep -E "^[a-z]{4,8}$" /usr/share/dict/words | gshuf -n4 | gsed 's/.*/\u&/' | tr -d '\n' | awk '{print $1}'; done | column

Current problems:

shuf requires brew install coreutils (since I'm on macOS).
sed requires brew install gnu-sed.
tr -d '\n' | awk '{print $1}' seems silly.
shuf: Is it truly random?

How can I make a short, yet readable the command line that works on "all" Unix flavours?

PS! I also found that by using SCOWL, which has a range from short word lists with common names to long word lists with obscure words, you can trade-off memorability Vs. entropy.

shuf: Is it truly random?

No. According to the GNU documentation,

  By default these commands use an internal pseudo-random generator initialized by a small amount of entropy, but can be directed to use an external source with the --random-source=file option.

For password generation you'll want to use --random-source=/dev/random.

gsed 's/.*/\u&/'

I've not seen this before. Nice trick, but it can be done in pure bash. The catch is that bash case modification is part of parameter expansion, so you have to store each word in a variable, and you'll need a loop. If you're not completely committed to having a one-liner then this is something which could be pulled out as an auxiliary function, and you might even find it useful elsewhere.

tr -d '\n' | awk '{print $1}'

awk seems a bit heavyweight given that the only reason for calling it is to append a newline. I'd replace with

tr -d '\n'; echo

In order to speed things up at the cost* of a small reduction in entropy, you could try to rewrite so that you only call shuf once, getting 128 words, and then split them into fours.

* (There's room for debate as to whether it's a cost or a benefit when you see whether using /dev/random slows things down while it tries to refill its entropy pool).


Loading an animation with an Ajax request I am not sure if this is the correct way to make an Ajax request and do the typical loading/processing GIF/animation while processing is happening and displaying the response.

HTML:

Photo Name : <input type='text' placeholder='Photo Name' name='photo-name' title='Photo Name' value='Untitled'>

                <img src="<?php echo 'tmp/' . $imgUpload->getFileName() . $imgUpload->getType() ?>" alt='photo'>

                Photo Tags : <input type='text' placeholder='Tags e.g. (#beach #park #dog)'>

                SaveCancel

CSS:

img { border: none; }
input { padding: 0.30em; }

.photo-setup {
    width: 600px;
    height: auto;
    margin: 0 auto;
    font-weight: 200;
    font-size: 0.9em;
    letter-spacing: 1px;
}

.setup-head {
    border-left: 1px solid #cacece;
    border-right: 1px solid #cacece;
    border-top: 1px solid #cacece;
    border-top-left-radius: 6px;
    border-top-right-radius: 6px;
    padding: 1em;
    overflow: hidden;
}

.photo-name { float: left; }

.photo-section {
    border: 1px solid #cacece;
    text-align: center;
}

.tag-section {
    padding: 1em 0 1em 1em;
}

.tag-section input {
    width: 81%;
    display: inline-block;
}

.ajax-loading {
    text-align: center;
    margin: 1em;
    display: none;
}

.response {
    display: none;
    text-align: center;
}

.commit-section {
    text-align: center;
}

.commit-section a {
    margin-left: 1em;
}

jQuery:

$(document).ready(function() {

    $('.save').on('click', function(e) {
        e.preventDefault();
        save();
    });

    $(document).ajaxStart(function() {
        $('.commit-section .save').hide();
        $('.ajax-loading').fadeIn();
    });

    $(document).ajaxComplete(function() {
        setTimeout(function() {
            $('.ajax-loading, .commit-section').hide();
            $('.response').fadeIn();
        }, 500);
    });

});

// collect all photo data and send to php for database storage
function save() {

    // regex to parse hashtags
    var regex = /[#]+[A-Za-z0-9-_]+/g;
    var name = $.trim($('.photo-name input').val());
    var tags = $.trim($('.tag-section input').val()).match(regex);
    var photo = $.trim($('.photo-section img').attr('src'));

    $.ajax({

        url: 'save_photo.php',
        method: 'POST',
        dataType: 'html',
        data: {
            title: name,
            hash_tags: tags,
            filename: photo
        }

    })
    .done(function(response) {
        $('.response').html(response);
    })
    .fail(function() {
        alert('Something went wrong!')
    });
}

As far as basic implementation. You've got it down pat. That being said, I do have a few suggestions that could allow your app to run more fluently. I assume that you are uploading these photos and that they aren't photos already on the server being accessed.

If my above assumption is true, I highly suggest you use the <input type='file' />. This will display the name of the file and will better maintain the file object for you when you pass it to the server. That being said, with the current implementation you actually don't upload any photos. All you do is give a name, but you don't physically package up the photo data and send it to the server to be processed.

Mizech makes a good point in his comment, though. Typically, you don't want to pass special characters unless you absolutely have to. That being said, the tagging system means you will having # showing up so why force the user to input it? Why not just have them input the tag without it? On the server side you can take the input and append the # in front because you know it should be there. It's about only having to do as much work as necessary and if you have to regex before sending then it would be easier to just send and then add it on the server.


Prim's algorithm in CoffeeScript I'm just starting to learn CoffeeScript and am trying to work through simple examples, this one being Prim's algorithm. I'd like feedback on everything but especially on making this script take advantage of the features of CoffeeScript most effectively.

nodes_dir = {
0 : [1],
1 : [0, 2, 3, 5],
2 : [1, 4],
3 : [1, 4, 5],
4 : [2, 3],
5: [1, 3]
}

edges_dir = {
0: {1:6}
1: {2:1, 3: 2, 5:0, 0:6},
2: {1:1, 4:4},
3: {1:2, 4:5, 5:3},
4:{2:4, 3:5},
5:{3:3, 1:0},
}

MAX_WEIGHT = 10000
NUM_NODES = 6
EDGE_WEIGHTS = [99, 2, 2, 1, 1, 5, 10]

get_min_key_value = (obj) ->
    if Object.keys(obj).length < 1
        return [-1, -1]
    min_k = MAX_WEIGHT
    min_v = MAX_WEIGHT
    for k, v of obj
        if v < min_v
          min_v = v
          min_k = k
    delete obj[min_k]
    [min_k, min_v]

# Prims algorithm as implemented in Skiena p 195
prim = (nodes_dir, edges_dir) ->
   # record-keeping arrays
   intree = []
   distance = []
   link = []
   parent = []

   # set default values for record-keeping arrays
   for i in [1..NUM_NODES]
       intree.push -1
       distance.push MAX_WEIGHT
       link.push -1
       parent.push -1

   links = []
   link_add = -1
   current_node = 0

   while intree[current_node] < 0
      # add new node and new edge to record of edges/nodes
      intree[current_node] = 1
      links.push link[current_node] if link[current_node] > 0

      # update any distances to tree that have become shorter with addition of new node
      p = edges_dir[current_node]
      for k,v of p
          weight = EDGE_WEIGHTS[v]
          if distance[k] > weight and intree[k] < 0
              distance[k] = weight
              link[k] = v
              parent[k] = current_node

      # determine which node is now closest to tree and add it
      dist = MAX_WEIGHT
      for i in [0..NUM_NODES-1]
          if intree[i] < 0 and dist > distance[i]
              dist = distance[i]
              current_node = i
              link_add = link[i]

   links

f = prim(nodes_dir, edges_dir)
alert f

This already seems pretty good considering you just started learning coffeescript.

for k, v of obj
     if v < min_v
       min_v = v
       min_k = k

Personally, I'm a huge fan of comprehensions. I would change the above to the following.

for k, v of obj when v < min_v
    min_v = v
    min_k = k

Your declarations at the top can be refined. No need for brackets/commas.

nodes_dir = {
0 : [1],
1 : [0, 2, 3, 5],
2 : [1, 4],
3 : [1, 4, 5],
4 : [2, 3],
5: [1, 3]
}

Can be simplified to just

nodes_dir =
    0: [1]
    1: [0, 2, 3, 5]
    2: [1, 4]
    3: [1, 4, 5]
    4: [2, 3]
    5: [1, 3]

Regarding the following

links.push link[current_node] if link[current_node] > 0

Don't lookup the value in the array based on the index twice.

linkNode = link[current_node]
links.push linkNode if linkNode > 0

Lastly,

intree = []
distance = []
link = []
parent = []

# set default values for record-keeping arrays
for i in [1..NUM_NODES]
    intree.push -1
    distance.push MAX_WEIGHT
    link.push -1
    parent.push -1

I would suggest creating an object that has these four arrays as properties, as well as putting that initialization code (the for loop) inside it's own method. This would add to clarity/reusability. Also, put all this code inside a coffeescript class declaration, which has a huge host of benefits.


Bugs - Program that Creates a Directory in C I have the following code that creates a directory in C. I tested it multiple times, reviewed it a few times and it looks fine at the moment.

What I really wanna know is what bugs does it have and how can I fix them ? (comparing it with the mkdir command from linux)

#include "ourhdr.h"
#include <sys/stat.h>
#include <limits.h>
#include <dirent.h>

int main(int argc, char **argv)
{

    if(argc != 2)
    {
        printf("./i78 /locatie/nume_director sau ./i78 nume_director\n");
        return 1;
    }

    char buff[PATH_MAX];
    strcpy(buff,argv[1]);

    if (mkdir(buff,S_IRWXU|S_IRWXG|S_IRWXO)<0)
    {
    printf("%s: cannot create directory '%s': File exists\n",argv[0],buff);
    }
    else

    printf("\n Creare director [%s]\n",buff);

}

There is a potential buffer overflow - the length of argv[1] may be up to ARG_MAX, which is likely larger than PATH_MAX.

The "File Exists" message may be misleading, mkdir can also fail because of permissions, out of space, invalid characters in the name, ...

It's good practice to put { } around all conditionals, including the else.


Stopwatch interface OOP (Vanilla JS) I'm learning OOP and the first exercise of the course was to made a Stopwatch class. I did it really fast and decided to make an interface for this little app.
I created the interface. Basically I want to know if the aproach that I used is fine, if I needed to create another class to control the ui, etc. I'm really lost with this OOP-UI integration thing.
The app is available in https://nacho-p5.github.io/
Here's the code:
Repo: https://github.com/nacho-p5/nacho-p5.github.io
index.html
<!DOCTYPE html>

    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <link rel="stylesheet" href="normalize.css">
    <link href="https://fonts.googleapis.com/css2?family=Lato:wght@100;300;700&display=swap" rel="stylesheet">
    <link rel="stylesheet" href="style.css">
    <title>Online Stopwatch</title>

        0.00

            <button type="submit" id="btnStart" class="btn">Start</button>
            <button type="submit" id="btnStop" class="btn">Stop</button type="submit">
            <button type="submit" id="btnReset" class="btn">Reset</button type="submit">

stopwatch.js
// SW class
function Stopwatch() {
    let counter = 0;
    let intervalID = 0;
    const screen = document.getElementById('sw-screen');

    const addMiliseconds = () => {
        counter += 0.01
        updateScreen();
    };
    const updateScreen = () => {
        screen.innerHTML = counter.toFixed(2);
    }

    // start()
    this.start = function() {
        if (intervalID != 0) {
            throw new Error('Stopwatch has already started.')
        }
        intervalID = setInterval(addMiliseconds, 10);
    };
    // stop()
    this.stop = function() {
        if (intervalID == 0) {
            throw new Error('Stopwatch has already stoped.')
        }
        clearInterval(intervalID);
        intervalID = 0;
    };
    // reset()
    this.reset = () => {
        if (intervalID != 0) {
            this.stop();
        };
        counter = 0;
        updateScreen();
    };
    // duration()
    Object.defineProperty(this, 'duration', {
        get: function() {
            console.log(intervalID);
            return counter
        }
    });
};

// UI class
function UI(arrayOfbuttons) {
    this.toggleClass = function(elem) {
        arrayOfbuttons.forEach(e => e.classList.remove('activeBtn'))
        elem.classList.add('activeBtn');
    };
    this.removeClass = function() {
        arrayOfbuttons.forEach(e => e.classList.remove('activeBtn'))
    };
};

// HTML components
const screen = document.getElementById('sw-screen');
const btnStart = document.getElementById('btnStart');
const btnStop = document.getElementById('btnStop');
const btnReset = document.getElementById('btnReset');

// Initialize classes
const sw = new Stopwatch();
const ui = new UI([btnStart, btnStop]);

// Add event listeners
btnStart.addEventListener('click', function() {
    sw.start();
    ui.toggleClass(this);
});
btnStop.addEventListener('click', function() {
    sw.stop();
    ui.toggleClass(this);
});
btnReset.addEventListener('click', function() {
    sw.reset();
    ui.removeClass();
});

Welcome to Code Review! Hopefully you find the feedback here and in other posts useful!
Good things
The code makes good use of const for items that shouldn't be re-assigned (e.g. screen, btnStart, screen, etc.) and let for re-assignable values (e.g. counter, intervalID).
Suggestions
This code uses some ES6 features like arrow functions - thus it could use the class syntax. Until then, each instance of StopWatch has methods defined using this.[method]. This works but isn't as efficient as using the prototype. Such a change might likely require changing local variables to instance variables (e.g. screen => this.screen).
The naming of methods on UI could be more appropriate - e.g. toggleClass would be more appropriate as toggleActiveClass because it toggles the activeBtn class, and removeClass would be more appropriate as removeActiveClass for the same reason.
I see that the Error is thrown when clicking the button with text Start twice, which makes sense, but perhaps the click event handler should only call the function sw.start (as well as ui.toggleClass(this)) if intervalID === 0 - if that is a case then perhaps a public method to determine if the Stopwatch is started is necessary.
I noticed the HTML for the buttons labeled Stop and Reset have attributes on the close tag:

<button type="submit" id="btnStop" class="btn">Stop</button type="submit">
<button type="submit" id="btnReset" class="btn">Reset</button type="submit">

Setting attributes on the closing tags will either be ignored by the browser, or could lead to an error - see answers to this SO question for more of an explanation.


Rijndael S-box for a string of bits My question is more of a "can someone give me tips" rather than "can you do it for me"

I have been writing a C code implementation that takes a 32-char String in the form of binary (supposed to act as a 32-bit instruction) and divides it to four 8-char strings. Those four strings are converted to Hex and fed to a small code program that returns the Rijndael S-box Forward and Inverse equivalent.

When I showed the code to my supervisor, he asked me to optimize the code I have. Even though after many iterations of changing different parts of the code and reducing the performance time slightly, I do not have ideas of how to improve it. So, the code is below, and I would be grateful for any tips and suggestions of how I can improve the performance of the code below. (Also, if you could suggest how to improve the amount of memory the code is using, that would be fabulous as well.)

#include <stdio.h>
#include <string.h>

unsigned char mulinv(unsigned char in) {
     //  multiplicative inverse in Rijndael's finite field.
    unsigned char k = 1, l = 1;
    //GF(2^8) = GF(2)[x]/(x8 + x4 + x3 + x + 1)
    //this takes the number of times I have to itterate in Rijndael's
    //finite field before I get my equivelant
    do {
        // equals k * 3
        k = k ^ (k << 1) ^ (k & 0x80 ? 0x1B : 0);
        // equals l / 3
        l ^= l << 1; l ^= l << 2; l ^= l << 4; l ^= l & 0x80 ? 0x09 : 0;
        } while (k != in);
        return l;
}

unsigned char makesbox(unsigned char in) {
        unsigned char s, x, l;
        l = mulinv(in);
        // affine transformation
        s = l ^ ((l << 1) | (l >> 7)) ^ ((l << 2) | (l >> 6)) ^ ((l << 3) | (l >> 5)) ^ ((l << 4) | (l >> 4));
        x = s ^ 0x63;
        return x;
}

unsigned char invsbox(unsigned char in) {
        unsigned char x;
         // affine transformation
        x = ((in << 1) | (in >> 7)) ^ ((in << 3) | (in >> 5)) ^ ((in << 6) | (in >> 2)) ^ 0x05;
        x = mulinv(x);
        return x;
}

void instSbox(unsigned char inst []) {
//        printf("Inst is : ");
        int i = 0;
//        for (i = 0; i < 32; i++){
//                printf("%c", inst[i]);
//        }
        unsigned char in[9];
        printf("\n---------------|Byte|SBox\n");
        for (i = 0; i < 33; i++){
                if (i%8 == 0 && i != 0) {
                        in[8] = '\0'; //NUL terminate the string.
                        unsigned char t = (unsigned char) strtol(in, NULL, 2);
                        printf("0x%X ", t);
                        printf("0x%X ", makesbox(t));
                        printf("0x%X \n", invsbox(t));
        }
        in[i%8] = inst[i];
        printf("%c ", in[i%8]);
    }
}

void main(){

    unsigned char inst [32] = "10101010101010101111111100111101"; // 32 bits (length of instructions)
    instSbox(inst);
}

The indentation seems to have gone wrong. This may not be entirely your fault - the Stack Exchange backend converts tabs to spaces - but it makes it harder than it should be to match { and } on some blocks.

The obvious place to look for optimisation is mulinv. One way to speed it up is a logarithm table - basically, do the loop once to fill a table with all 256 cases, and then just look up values in the table. More sophisticated approaches are to use Euclid's algorithm for GCD on elements of \$GF(2^8)\$, or to consider the field \$GF(2^8)\$ as a tower of field extensions, \$GF(2)[x]/p(x)/q(x)\$, which makes it fairly simple to reduce the inversion to one in \$GF(16)\$. You can find more detailed explanations on another site from this network in a rather unusual challenge format: https://codegolf.stackexchange.com/q/9276/194

The rotations for the affine transforms also look ripe for optimisation. If you use a 16-bit integer then

        s = l ^ ((l << 1) | (l >> 7)) ^ ((l << 2) | (l >> 6)) ^ ((l << 3) | (l >> 5)) ^ ((l << 4) | (l >> 4));

can be replaced with

        uint16_t l16 = l;
        uint16_t t = l16 ^ (l16 << 1) ^ (l16 << 2) ^ (l16 << 3) ^ (l16 << 4);
        s = (unsigned char)(t ^ (t >> 8));

and similarly in invsbox.

Finally, a point on names. inst is completely cryptic to me with the context given. Perhaps you could use the full word, comment on the meaning, or consider whether in fact whatever word it abbreviates is inherently what is operated on or merely a specific application.


Anonymous Classes for Listeners While working with Android GUI my approach to adding Listeners for GUI elements is usually:

Button helpBt = (Button) findViewById(R.id.help_button);
helpBt.setOnClickListener(new View.OnClickListener() {
   /* Code */
});

Currently, I am at learning stage. However, will this approach be maintainence 'friendly' as I get working with more complex apps? If not, then what are suitable alternatives?

It depends on how much /* Code */ there is. If it's just a few lines, that's one thing. If it gets too large though, the reader will lose sight of the fact that you're creating an anonymous listener. I would say that if the anonymous listener is more than 5, or maybe 10 lines then you should use an inner class instead.

There comes a point where even an inner class is really too large though.

In general, questions about "When is  too big?" are highly subjective, but failing to ask them, and failing to act on the answers leads to hard to read, difficult to maintain code.

Also, as biovamp pointed out, if you want to reuse the code, then you need to create a regular class.


Is this a proper way of "loading" views in PHP? As a learning exercise, I'm developing my own PHP framework. I'm looking for a way to "load views" (kinda like CodeIgniter does it), without polluting my general scope.

I came up with the following, basic, example:

$data['test'] = 'Hello world';

$str = load_view($data, true);
echo $str;

function load_view($data, $store = false) {
    extract($data);

    ob_start();
    include('view.php');

    if($store)  return ob_get_clean();
    else        ob_end_flush();
}

// End of file

view.php

<?php echo $test; ?>

I could also use load_view($data), which would output the contents of view.php immediately.

Edit: I'm mostly worried about performance. As Peter pointed out, I'm aware that the function should be a class method that's seperate from the logic.

I agree that its a better pattern to wrap your view logic in a class.

Here is some code I whipped together tonight - It represents the simplest class I could devise that contains the minimum logic that I require in a view:

Includes - Ability to include other views
Captures - Ability to easily capture content within your view
Layouts - Ability to inject data into a re-usable layout template
Fetching - Ability to fetch view output instead of sending it to output buffer
data - Ability to access the resulting data once the view is finished

[edit] Refactored per conversation in comments regarding passing $data by reference.

View.php

<?php
/**
 * Simple view class that supports includes, capturing, and layouts, as well
 * as retrieving rendered view content and resulting data.
 *
 * *NOTE* When a view uses a layout, the output of the view is ignored, as
 *        as the view is expected to use capture() to send data to the layout.
 *
 * @author David Farrell <DavidPFarrell@gmail.com>
 */
class View implements ArrayAccess
{
    /**
     * View file to include
     * @var string
     */
    private $file;

    /**
     * View data
     * @var array
     */
    private $data;

    /**
     * Layout to include (optional)
     * @var string
     */
    private $layout;

    /**
     * Constructor
     *
     * @param string $file file to include
     */
    public function __construct($file)
    {
        $this->file = $file;
    }

    /**
     * render Renders the view using the given data
     *
     * @param array $data
     * @return void
     */
    public function render($data)
    {
        $this->data = $data;
        $this->layout = null;

        ob_start();

        include ($this->file);

        // If we did not set a layout
        if (null === $this->layout)
        {
            // flush view output
            ob_end_flush();
        }
        // We set a layout
        else
        {
            // Ignore view output
            ob_end_clean();

            // Include the layout
            $this->include_file($this->layout);
        }
    }

    /**
     * fetch Fetches the view result intead of sending it to the output buffer
     *
     * @param array $data
     * @return string The rendered view content
     */
    public function fetch($data)
    {
        ob_start();
        $this->render($data);
        return ob_get_clean();
    }

    /**
     * get_data Returns the view data
     *
     * @return array
     */
    public function get_data()
    {
        return $this->data;
    }

    /**
     * include_file Used by view to include sub-views
     *
     * @param string $file
     * @return void
     */
    protected function include_file($file)
    {
        $v = new View($file);
        $v->render($this->data);
        $this->data = $v->get_data();
    }

    /**
     * set_layout Used by view to indicate the use of a layout.
     *
     * If a layout is selected, the normal output of the view wil be
     * discarded.  The only way to send data to the layout is via
     * capture()
     *
     * @param string $file
     * @return void
     */
    protected function set_layout($file)
    {
        $this->layout = $file;
    }

    /**
     * capture Used by view to capture output.
     *
     * When a view is using a layout (via set_layout()), the only way to pass
     * data to the layout is via capture(), but the view can use capture()
     * to capture text any time, for any reason, even if the view is not using
     * a layout
     *
     * @return void
     */
    protected function capture()
    {
        ob_start();
    }

    /**
     * end_capture Used by view to signal end of a capture().
     *
     * The content of the capture is stored under $name
     *
     * @param string $name
     * @return void
     */
    protected function end_capture($name)
    {
        $this->data[$name] = ob_get_clean();
    }

    /* ArrayAccess methods */
    public function offsetExists($offset)      { return isset($this->data[$offset]); }
    public function offsetGet($offset)         { return $this->data[$offset]; }
    public function offsetSet($offset, $value) { $this->data[$offset] = $value; }
    public function offsetUnset($offset)       { unset($this->data[$offset]); }

}

run.php

<?php
require "View.php";

$v = new View('view_main_simple.php');
$fetch = $v->fetch(array('message' => 'Hello, world'));
print("Fetch result: {$fetch}\n");

$v = new View('view_main_complex.php');
$v->render(array('one' => 1, 'two' => 2, 'rows' => array('a','b','c')));

$data = $v->get_data();

print("\n");
var_export($data);

view_main_simple.php

The message is: <?php echo $this['message'] ?>

view_main_complex.php

<?php $this->set_layout('view_layout.php') ?>
<?php $this->capture() ?>
    one=<?php echo $this['one'] ?>

    <?php $this->include_file('view_include.php') ?>
    three=<?php echo $this['three'] ?>

<?php $this->end_capture('body') ?>

view_include.php

two=<?php echo $this['two'] ?>

<?php $this['three'] = 3 ?>

    <?php foreach($this['rows'] as $row) { ?>

<?php echo $row ?>

    <?php } ?>

view_layout.php

<?php echo $this['body'] ?>

Program Output

Fetch result: The message is: Hello, world

    one=1

    two=2

a

b

c

    three=3

array (
  'one' => 1,
  'two' => 2,
  'rows' =>
  array (
    0 => 'a',
    1 => 'b',
    2 => 'c',
  ),
  'three' => 3,
  'body' => '   one=1

    two=2

a

b

c

    three=3

',
)


Generating all possible combinations of a string using iteration I am generating all possible combinations from a string concatenated. I have a \$O(n*2^n)\$ solution.

public static String[] combinations(String[] array) {
    String[] res = new String[(1 << array.length) - 1];
    int k = 0;
    int x = 1;
    for (int i = array.length - 1; i >= 0; --i) {
        res[k++] = array[i];
        for (int j = 1; j < x; ++j) {
            res[k++] = array[i] + res[j - 1];
        }
        x *= 2;
    }
    return res;
}

How can this be improved?

Minor changes

As @coderodde already stated, there is not much to improve, nonetheless a few minor things:

You can reduce the scope of the variables x (which has always the same value as k prior entering the inner loop) and k to the inner/outer loop.
for (int i = array.length - 1; i >= 0; --i) can be written as for (int i = array.length; --i >= 0;).
Your inner loop starts at j = 1 but only uses j - 1, you can start the loop at 0 to avoid the subtraction.

Depending on how nulls should be handled, you could use res[k++] = array[i].toString() to throw a NPE if null elements are present. If nulls are permitted, I would prefer that the string "null" is added to the resulting array rather than a null reference.

You could start k and j at -1 and use the (in theory) more efficient preincrement operator instead of the postincrement operator.

Possible implementation (not using preincrement for kand j as I think it makes the code more difficult to read):

public static String[] combinations(String[] array) {
    String[] res = new String[-1 >>> -array.length];
    for (int i = array.length, k = 0; --i >= 0;) {
        String s = res[k] = array[i].toString();
        for (int j = 0, x = k++; j < x;)
            res[k++] = s + res[j++];
    }
    return res;
}

Alternative implementation

Your current implementation supports arrays with a maximum length of 30, you could provide a method #combination(String[] array, long index) to support arrays with up to 64 elements (or a larger datatype than long to support way larger input arrays).

public static String combination(String[] array, long index) {
    // Could iterate twice to determine length (and coder) first
    // to avoid resizing buffer (and could share the resulting
    // byte array), kept this way for simplicity.
    if (array.length > 64 || index == -1 || index + 1 >>> array.length != 0)
        throw new IllegalArgumentException();
    index = index + 1 << -array.length;
    StringBuilder sb = new StringBuilder();
    for (int i = -1; index != 0;) {
        int nlz = numberOfLeadingZeros(index);
        sb.append(array[i += nlz + 1]);
        index = index << nlz << 1;
    }
    return sb.toString();
}


Unit tests for a single JavaScript function Here is the question: What is the better way to unit test this specific function?  Should all the test items be included in one test or is it better to break up the tests so each test is testing for something more specific?

Function being tested:

$scope.saveFailure = function (response) {
        var errs = [];
        response.data.forEach((entry) => errs.push("options." + entry.dataField));
        if (errs.length > 0) $scope.$emit('datafields:errors', errs);

        $scope.gotoTop();
        $scope.processing = false;
    };

Method 1: Multiple Unit Tests

var mockFailureResponse;

beforeEach(function () {
    mockFailureResponse = {
        data: [],
    };
});

it('saveFailure should set processing to false', function () {
    $scope.processing = true;
    $scope.saveFailure(mockFailureResponse);
    expect($scope.processing).toBe(false);
});
it('saveFailure should call goToTop()', function () {
    spyOn($scope, 'gotoTop');
    $scope.saveFailure(mockFailureResponse);
    expect($scope.gotoTop).toHaveBeenCalled();
});
it('saveFailure should emit datafield errors when present', function () {
    spyOn($scope, '$emit');
    mockFailureResponse = {
        data: [{dataField: "field"}],
    };
    $scope.saveFailure(mockFailureResponse);
    expect($scope.$emit).toHaveBeenCalledWith('datafields:errors', ['options.field']);
});
it('saveFailure should not emit datafield errors non are present', function () {
    spyOn($scope, '$emit');
    $scope.saveFailure(mockFailureResponse);
    expect($scope.$emit.calls.count()).toEqual(0);
});

Method 2: Single Unit Test

it('saveFailure should handle failed requests', function () {
    spyOn($scope, '$emit');
    let mockFailureResponse = {
        data: [],
    };
    $scope.saveFailure(mockFailureResponse);
    expect($scope.$emit.calls.count()).toEqual(0);

    mockFailureResponse = {
        data: [{ dataField: "field" }],
    };
    $scope.saveFailure(mockFailureResponse);
    expect($scope.$emit).toHaveBeenCalledWith('datafields:errors', ['options.field']);

    spyOn($scope, 'gotoTop');
    $scope.saveFailure(mockFailureResponse);
    expect($scope.gotoTop).toHaveBeenCalled();

    $scope.processing = true;
    $scope.saveFailure(mockFailureResponse);
    expect($scope.processing).toBe(false);
});

You can use array.map to create errs instead of creating an array and pushing to it.

const errs = response.data.map(entry => `options. ${entry.dataField}`)

With that out of the way, there's only two things that can happen with this code:

No errors emitted, app goes to top, processing is false.
Error emitted, app goes to top, processing is false.

Suggesting you create 2 tests, one for each branch and testing all 3 things. The function is small enough, there's no need to get too complicated.


Having models which inherit from one base model I have a model such as this below that acts as a base model. Several other models inherit from this one since they will all share common properties. Would an approach like this fit best practices or would it be better to go another route. I'm aware that composition is always considered a best practice.

public class BaseServiceResponse
{
    #region Properties

    public Guid PatientId { get; set; }
    public Guid PracticeId { get; set; }
    public ServiceStatusCodes Status { get; set; }
    public string Message { get; set; }
    public string ReturnUrl { get; set; }

    #endregion

    #region Constructors

    public BaseServiceResponse()
    {
        InitMembers();
    }

    public BaseServiceResponse(Guid patientId)
    {
        this.PatientId = patientId;
    }

    #endregion

    #region Private Methods

    private void InitMembers()
    {
        this.PatientId = Guid.Empty;
        this.PracticeId = Guid.Empty;
        this.Status = 0;
        this.Message = string.Empty;
        this.ReturnUrl = string.Empty;
    }
    #endregion
}

I then have several other models which inherit from it such as this one

public class FullReportResponse : BaseServiceResponse
{
    #region Properties

    public List<string> ReportNames;
    public string SomeOtherProp;

    #endregion

    #region Constructors

    public FullReportResponse(Guid patientId, List<string> reportNames)
    {
        this.PatientId = patientId;
        this.ReportNames = reportNames;
    }

    #endregion
}

I'm aware that composition is always considered a best practice.

I wouldn't say that it's always the best choice. Though I guess that's not what you're saying, since you chose inheritance anyway.

In general, best practices are just guidelines to keep in mind when designing and implementing your code.

In this specific case, I think that inheritance is the right solution.

#region Properties

I don't see the need to have these kinds of #regions all over your code. Your classes aren't humongous so normal code folding will work just fine.

InitMembers();

Why is this code in a separate method? Unless you have a good reason for that, just put it directly into the constructor.

public BaseServiceResponse(Guid patientId)
{
    this.PatientId = patientId;
}

This means that the other members aren't going to be initialized. You should probably delegate from one constructor to the other using : this() to fix this.

this.PatientId = Guid.Empty;
this.PracticeId = Guid.Empty;
this.Status = 0;
this.Message = string.Empty;
this.ReturnUrl = string.Empty;

The empty GUID and 0 are the default values for Guid and an enum, respectively, so you don't need to initialize them to that. Though it might make sense to do it anyway for clarity.

Initializing the string properties to empty string makes it less clear that the value is uninitialized and could potentially hide bugs.

public List<string> ReportNames;
public string SomeOtherProp;

These should be properties too, not public fields. And when you make ReportNames a field, consider making the setter private.

public FullReportResponse(Guid patientId, List<string> reportNames)
{
    this.PatientId = patientId;
    this.ReportNames = reportNames;
}

Consider making a copy of reportNames here. Otherwise, it's easy to write buggy code that passes the list to FullReportResponse and then reuses it for other purposes, which will change the FullReportResponse.

Also, consider accepting IEnumerable<string> to be more flexible.


Responsive search bar implementation using OOCSS I am implementing a search bar here. I have finished basic UI. I have been recently developed a taste for scalable and robust HTML/CSS. Therefore I am closely following things like BEM and OOCSS

Am I on the right path? Also, how can I make the UI responsive?

index.html

  <form class="search-box">

    <input class="search-box-input" type="text" placeholder="Search"/>

      Foo Bar

  </form>

app.css

* {
  margin: 0;
  box-sizing: border-box;
  font-family: 'Roboto', sans-serif;
}

body {
  background: #343d46;
}

#container {
  width: 500px;
  margin: auto;
  margin-top: 30px;
  background: gray;
  color: black;
}

.search-box {
  width: 100%;
  position: relative;
  background: inherit;
  color: inherit;
  height: auto;
}

.search-box:hover .icon-search {
  opacity: 1;
}

.search-box-input {
  background: inherit;
  color: inherit;
  height: 50px;
  font-size: 18px;
  width: 100%;
  padding: 10px;
  padding-left: 40px;
  display: inline-block;
  border: none;
}

.search-box-input:focus {
  outline: none;
}

.search-box-icons {
  position: absolute;
  display: inline-block;
  margin: auto;
  top: 16px;
  left: 10px;
  width: auto;
}

.icon-search {
  color: inherit;
  font-size: 16px;
  display: inline;
  cursor: pointer;
  opacity: 0.4;
}

.search-box-autocomplete {
  position: absolute;
  background: #fff;
  width: 100%;
  margin: 0;
  border-top: 1px solid gray;
}

Markup

You're using way more than you need.  You have an unnecessary container element, and 2 empty elements for displaying a purely decorative icon.  This should be all the markup you need to get the same effect:

<form class="search-box">
    <input class="search-box-input" type="search" placeholder="Search"/>
    <div class="search-box-autocomplete" style="display: none;">
        Foo Bar
    </div>
</form>

Placeholder text is not a replacement for label text.  It's supposed to be for providing an example of the type of content you're looking for.

Strange that you're using the html5 placeholder property, but you're not using the html5 input type of search.

CSS

I do not subscribe to the BEM methodology, so I cannot comment as to how well you've followed it;  it always looks overly verbose to me.

As I've already stated, your icon is purely decorative.  It has no place in the markup.  The most appropriate location for it is as a pseudo element on the label for the search field (which doesn't exist here) or the form element.

The following CSS:

#container {
  width: 500px;
  margin: auto;
  margin-top: 30px;
  background: gray;
  color: black;
}

.search-box {
  width: 100%;
  position: relative;
  background: inherit;
  color: inherit;
  height: auto;
}

Can be reduced to this:

.search-box {
  width: 500px;
  margin: 30px auto 0 auto;
  background: gray;
  color: black;
  position: relative;
}

Usability

The entire color scheme has extremely low contrast.

The styling that browsers give when the input element has focus is considered a usability feature.  If it doesn't fit into your design, you're supposed to adjust it to fit, not remove it.

There's no submit button.  There are users who don't understand that they can just hit enter to submit a form.


Building a tree in python Please can someone confirm if this code is the best way to build the following tree

           0
         /   \
        1      1
       / \    / \
      3  8    4  7

class Node():
    def __init__(self, value, right = None, left= None):
        self.value = value
        self.right = right
        self.left = left
    def addNode (self, list_parent_nodes, child_value):
        if self.value >=0:
            if len(list_parent_nodes) > 1:
                if list_parent_nodes[0] == 0:
                    self.left.addNode (list_parent_nodes[1:], child_value)
                else:
                    self.right.addNode (list_parent_nodes[1:], child_value)
            else:
                if list_parent_nodes[0] == 0:
                    self.left = Node(child_value)
                else:
                    self.right = Node(child_value)
    def printTree(self,level=0):
        if self.value >=0:
            output = self.value
            print("  "*level + str(output))
            if self.left: self.left.printTree(level+1)
            if self.right: self.right.printTree(level+1)

g = Node(0)
g.addNode ([0], 1)
g.addNode ([1], 1)
"""Graph looks like
   0
  / \
 1   1
 """

g.addNode ([0,0], 3)
g.addNode ([0,1], 8)
g.addNode ([1,0], 4)
g.addNode ([1,1], 7)
"""Graph looks like
           0
         /   \
        1      1
       / \    / \
      3  8    4  7
"""
g.printTree()

```

Please read over PEP 8 as your naming style is unidiomatic, and would cause programmers to be confused when seeing your code in the wild.
I would move getting and setting a node by index into their own functions.
if self.value >= 0 only hinders your code, the value -1 is perfectly valid in binary trees. It also means that you're limiting value to types that can be compared to integers, meaning you can't enter strings.
Using recursion in addNode is a good idea, but I find the non-recursive alternate to be easier to understand.
Your code doesn't care if I enter addNode([0, 2], 1). This seems fishy as if this were a Trinary Tree that would mean something completely different. I recommend raising a ValueError in this case.
Your printTree is pretty good.
I'd change the level argument in printTree to be a keyword argument only. This is because then it's explicit that it's changing the level, and it's something normal code shouldn't accidentally change.
Personally I think if self.left is not None is better here than if self.left, but as Node can't be falsy it doesn't matter too much.
I've added some basic docstrings, defined in PEP 257.
I have also added some typing to your code.
By using typing and mypy I found that addNode can error when the specified path doesn't exist. It would be better in these cases to raise an error telling the user why.
You can use a if __name__ == '__main__': guard to prevent code running on import. This means if you later import this code in another module then the testing code won't run.

from __future__ import annotations

from typing import Any, Optional, Sequence

class Node:
    """Test binary tree."""

    def __init__(
            self,
            value: Any,
            right: Optional[Node] = None,
            left: Optional[Node] = None,
    ):
        """Initialize binary tree, with specified children nodes."""
        self.value = value
        self.right = right
        self.left = left

    def _get_index(self, index: int) -> Optional[Node]:
        """Get node via integer index."""
        if index == 0:
            return self.left
        elif index == 1:
            return self.right
        else:
            raise ValueError(f'Invalid index {index}')

    def _set_index(self, index: int, value: Node) -> None:
        """Set node via integer index."""
        if index == 0:
            self.left = value
        elif index == 1:
            self.right = value
        else:
            raise ValueError(f'Invalid index {index}')

    def add_node(self, parents: Sequence[int], value: Node) -> None:
        """Add the provided node to the tree."""
        node: Optional[Node] = self
        for index in parents[:-1]:
            if node is not None:
                node = node._get_index(index)
        if node is None:
            raise ValueError("Parent node doesn't exist.")
        node._set_index(parents[-1], value)

    def add_value(self, parents: Sequence[int], value: Any) -> None:
        """Add the provided value to the tree."""
        self.add_node(parents, Node(value))

    def print_tree(self, *, level: int = 0) -> None:
        """Print the tree."""
        print('  ' * level + str(self.value))
        for child in (self.left, self.right):
            if child is not None:
                child.print_tree(level=level + 1)

if __name__ == '__main__':
    tree = Node(0)
    tree.add_value([0], 1)
    tree.add_value([1], 1)
    r"""
    Graph looks like
       0
      / \
     1   1
     """

    tree.add_value([0, 0], 3)
    tree.add_value([0, 1], 8)
    tree.add_value([1, 0], 4)
    tree.add_value([1, 1], 7)
    r"""
    Graph looks like
               0
             /   \
            1      1
           / \    / \
          3  8    4  7
    """
    tree.print_tree()


Logging all API call traffic in Java servlets We have a few servlets like the one below, taking JSON request and produce JSON response. And the requirement is to log all API call traffic.

import org.slf4j.*;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
public class CommentServlet {
  private static final Logger LOG = LoggerFactory.getLogger(CommentServlet.class);
  public void postComment(HttpServletRequest request, HttpServletResponse response) throws IOException {
    String commentRequested = request.getParameter("comment");
    LOG.info(commentRequested);
    Object postResultModel = processComment(commentRequested);
    JsonUtil.writeResponse(response, postResultModel);
  }
  /**
  * doesn't reallty matter what processComment does
  */
  public Object processComment(String comment) {
    return new Object();
  }
}

import org.slf4j.*;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
public class JsonUtil {
    private static final Logger LOG = LoggerFactory.getLogger(JsonUtil.class);
    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
    public static void writeResponse(HttpServletResponse response, Object modelObject) throws IOException {
        String modelJson = OBJECT_MAPPER.writeValueAsString(modelObject);
        LOG.info(modelJson);
        response.setContentType("application/json");
        response.getWriter().write(modelJson);
    }
}

if you can mainly review how the logging is done, I'd like feedback on the general approach as much as whether there's any way to improve it.

private static final Logger LOG = LoggerFactory.getLogger(CommentServlet.class);

This is considered as bad practice ... of course there is an argument from the other side as well.

The right thing to use should be

private final Logger LOG = LoggerFactory.getLogger(getClass());

This basically helps you leverage the capabilities of polymorphism and use the same logger in inherited classes with its real name.


Low level string manipulation functions in C I'm working with a legacy program that does a bunch of markup manipulation, some of it higher-level and specific to the proprietary markup, some of it lower level string stuff.

I rewrote and documented the customized low level stuff (and used string.h stuff where applicable). I'm not not usually a C-coder, but I'm trying to learn.

Am I duplicating core functionality of a standard library?
Am I following conventions/best practices?
Is the code efficient?

string_plus.h

/**
 * @file
 *
 * A small library of additional low level string manipulation functions. If any
 * functionality is duplicated from any of the normal libraries, it is due to
 * ignorance.
 */

#ifndef STRING_PLUS_H
#define STRING_PLUS_H

#include <string.h>

/**
 * Returns 1 if the front of @p text matches @p search_key, else 0.
 */
int begins_with(char const *text, char const *search_key);

/**
 * Condenses a string buffer by sliding all contents at @p p2 down to @p p1. In
 * other words, anything in between @p p1 and @p p2 is deleted. Returns @p p1.
 */
char *condense_buffer(char *p1, char *p2);

/**
 * Modifies @p text, removing all occurrences of @p c. Returns @p text.
 */
char *remove_char_occurrences(char *text, char const c);

/**
 * Inserts @p content into a buffer at @p p by shifting the buffer's data by @p
 * content's size. Returns a pointer at the end of the newly modified buffer.
 *
 * @warning Caller is responsible for guarding against a buffer overrun.
 */
char *insert_content(char *p, char const *content);

#endif /* Header load-once guard clause */

string_plus.c

#include "string_plus.h"

int begins_with(char const *text, char const *search_key) {
  int result = 1;
  while (*search_key != '\0' && (result = *search_key++ == *text++));
  return result;
}

char *condense_buffer(char *p1, char *p2) {
  while (*(p2-1) != '\0') *p1++ =*p2++;
  return p1;
}

char *remove_char_occurrences(char *text, char const c) {
  char *lead = text;
  char *tail = text;
  while (*lead != '\0') {
    while (*lead == c) ++lead;
    *tail++ = *lead++;
  }
  *tail = *lead;
  return text;
}

char *insert_content(char *p, char const *content) {
  int content_len = strlen(content);
  for (char *end = p + strlen(p); end >= p; end--) *(end + content_len) = *end;
  while (*content != '\0') *p++ = *content++;
  return p;
}

test functions

void test_begins_with() {
  Assert(begins_with("Theory", "The") == 1);
  Assert(begins_with("theory", "The") == 0);
  Assert(begins_with("T", "The") == 0);
  Assert(begins_with("Theory", "") == 1);
}

void test_condense_buffer() {
  char buff[] = "hello world";
  condense_buffer(&buff[7], &buff[10]);
  Assert(strcmp("hello wd", buff) == 0);
}

void test_remove_char_occurrences() {
  char s1[] = "hello world";
  Assert(strcmp("hello world", remove_char_occurrences(s1, 'x')) == 0);

  char s2[] = "hello world";
  Assert(strcmp("heo word", remove_char_occurrences(s2, 'l')) == 0);
}

void test_insert_content() {
  char buff[50] = "hello world";
  char *p = insert_content(&buff[5], "BRAVENEW");
  Assert(strcmp("helloBRAVENEW world", buff) == 0);
  Assert(strcmp(" world", p) == 0);
}

condense_buffer is equivalent to strcpy. If you still wish to implement it, use an idiomatic C construct:

    while ((*p1++ = *p2++) != 0)
        {}

begins_with is equivalent to strncmp(text, key, strlen(key)). It might be argued though that strncmp approach requires a call to strlen and thus incurs performance penalty.
Do put the loop body on a separate line. Do not omit curly braces even for one-liners:

   for (char *end = p + strlen(p); end >= p; end--) {
       *(end + content_len) = *end;
   }


Is there a way to eliminate the repetition in this linq query? This is a query to return search results, but I feel it could be cleaned up a little.  I just don't know how.

var queryResult = (from r in dc.Retailers
                    where
                        (string.IsNullOrEmpty(firstName) ||
                        SqlFunctions.PatIndex(firstName.Trim(), r.FirstName.Trim()) > 0) &&
                        (string.IsNullOrEmpty(lastName) ||
                        SqlFunctions.PatIndex(lastName.Trim(), r.LastName.Trim()) > 0) &&
                        (string.IsNullOrEmpty(companyName) ||
                        SqlFunctions.PatIndex(companyName.Trim(), r.CompanyName.Trim()) > 0) &&
                        (string.IsNullOrEmpty(phone) ||
                        SqlFunctions.PatIndex(phone.Trim(), r.Phone.Trim()) > 0) &&
                        (string.IsNullOrEmpty(email) ||
                        SqlFunctions.PatIndex(email.Trim(), r.Email.Trim()) > 0) &&
                        (string.IsNullOrEmpty(city) ||
                        SqlFunctions.PatIndex(city.Trim(), r.City.Trim()) > 0) &&
                        (string.IsNullOrEmpty(zip) ||
                        SqlFunctions.PatIndex(zip.Trim(), r.Zip.Trim()) > 0) &&
                        (string.IsNullOrEmpty(state) ||
                        SqlFunctions.PatIndex(state.Trim(), r.State.Trim()) > 0) &&
                        (string.IsNullOrEmpty(country) ||
                        SqlFunctions.PatIndex(country.Trim(), r.Country.Trim()) > 0)
                    select r
    );

Update: For those confused by the query.  Here is another version, which is probably better because it only builds the where parts when necessary.  But I would still like to reduce the repetition.

var queryResult = (from r in dc.Retailers select r);
if (!string.IsNullOrEmpty(firstName))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(firstName.Trim(), ex.FirstName.Trim()) > 0);
if (!string.IsNullOrEmpty(lastName))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(lastName.Trim(), ex.LastName.Trim()) > 0);
if (!string.IsNullOrEmpty(companyName))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(companyName.Trim(), ex.CompanyName.Trim()) > 0);
if (!string.IsNullOrEmpty(phone))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(phone.Trim(), ex.Phone.Trim()) > 0);
if (!string.IsNullOrEmpty(email))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(email.Trim(), ex.Email.Trim()) > 0);
if (!string.IsNullOrEmpty(city))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(city.Trim(), ex.City.Trim()) > 0);
if (!string.IsNullOrEmpty(zip))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(zip.Trim(), ex.Zip.Trim()) > 0);
if (!string.IsNullOrEmpty(country))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(country.Trim(), ex.Country.Trim()) > 0);
if (!string.IsNullOrEmpty(state))
    queryResult = queryResult.Where(ex => SqlFunctions.PatIndex(state.Trim(), ex.State.Trim()) > 0);

Apply the common refactoring used for repeated code: extracting a method. In your case, creating the method won't be trivial, because you want to create a different Expression each time. That is, unless you use LINQKit:

public static IQueryable<T> WhereEqualsByPatIndex<T>(
    this IQueryable<T> query, string value, Expression<Func<T, string>> propertyExpression)
{
    if (string.IsNullOrEmpty(value))
        return query;

   return query.Where(
       ex => SqlFunctions.PatIndex(
           value.Trim(), propertyExpression.Invoke(ex, propertyExpression).Trim()) > 0);
}

Usage would be like this (that call to AsExpandable() is important):

dc.Retailers
  .AsExpandable()
  .WhereEqualsByPatIndex(firstName, ex => ex.FirstName)
  .WhereEqualsByPatIndex(lastName, ex => ex.LastName)


Shank Transformation to Lebniez series to find Pi recursively I found a simple series named the Lebniez series for finding Pi.
$$1 - \frac{1}{3} + \frac{1}{5} - \frac{1}{7} + \frac{1}{9} - \dots = \frac{\pi}{4}$$
I made this in Python. I then realised that because it is an infinite series, and it will give an approximation. Additionally the convergence rate is very slow.
Here is the program I made after learning about series acceleration through the Shank Transform:
def lebniez(n):
    pi = 0
    n = n + 1
    for i in range(1,n):
        if(i % 2 == 1):
            pi -= 1 / (2*i - 1)
        else:
            pi += 1/(2*i - 1)
    pi = abs(4 * pi)
    return pi

def accelrate(n,depth):
    if depth == 1:
        a = lebniez(n + 1)
        b = lebniez(n)
        c = lebniez(n-1)
        return (a*c - b*b)/(a + c - 2*b)
    a = accelrate(n + 1,depth - 1)
    b = accelrate(n,depth - 1)
    c = accelrate(n-1,depth - 1)
    return (a*c - b*b)/(a + c - 2*b)

I have applied the shank transformation and gotten a new series. After this I continuously apply it recursively. However using recursion has made it very slow. If I keep accelerating the function then after some depth the accuracy starts decreasing.


Calculating Euclidean distance Code: I have a list of ~15,000 Euclidean points which represent a connected graph. I'm trying to solve/approximate the traveling salesman problem with this graph.

Problem: Finding neighbors, i.e. calculating the distance between each point, is very slow.

The points are stored in a vector as City objects:

class City {
public:
    City(int id, int xCoord, int yCoord);

    int getCityID() const;
    int getXCoord() const;
    int getYCoord() const;

    void addNeighbor(const City &city);
    void removeNeighbor(int cityID);
    unsigned long getNeighborCount();
    Neighbor nearestNeighbor();
private:
    int id;
    int xCoord;
    int yCoord;
    std::vector<Neighbor> neighbors;
};

And neighbors are added like so:

void City::addNeighbor(const City &city) {
    int distance = Utilities::distanceSquared(*this, city);

    // Prevent cities listing themselves as neighbors
    if (distance > 0) {
        Neighbor neighbor(city.getCityID(), distance);
        neighbors.push_back(neighbor);
    }
}

And the distance is calculated by this function:

unsigned int Utilities::distanceSquared(City a, City b) {
    return static_cast<unsigned int>(
        pow(b.getXCoord() - a.getXCoord(), 2) +
        pow(b.getYCoord() - a.getYCoord(), 2)
    );
}

Finally, I test the speed of this operation like so:

// Get input
std::string filename = "tsp_example_3.txt";
std::vector<City> input = Utilities::readFile(filename);

for (int c1 = 0; c1 < input.size(); c1++) {
    for (int c2 = 0; c2 < input.size(); c2++) {
        input[c1].addNeighbor(input[c2]);
        // Utilities::distanceSquared(input[c1], input[c2]); // ~30 seconds
    }
}

When I simply calculate the distance between points (Utilities::distanceSquared()), it takes about 30 seconds (I'm not sure if this is good or bad for doing simple math on ~225,000,000 integers). When I try to use my addNeighbor() function, the program runs for at least ten minutes without completing.

Any ideas on how I might speed up this operation?

I see a few things that may help you improve your code.

Sort the input

If the input is sorted, we can assure that the City id for vectors is strictly non-decreasing.  I'd add these two things for that.

std::sort(input.begin(), input.end());

And then this member function:

bool City::operator<(const City &other) const {
    return id < other.id;
}

Don't repeat calculations

Unless there's something very strange, the distance from city A to city B is the same as the distance from city B to city A.  For that reason, we can populate the vectors two at a time:

for (int c1 = 0; c1 < input.size(); c1++) {
    for (int c2 = c1; c2 < input.size(); c2++) {
        input[c1].addNeighbor(input[c2]);
        input[c2].addNeighbor(input[c1]);
    }
}

This is also why I suggested to sort the input vector.  I would also suggest calculating the distance once and then using it for both vector additions.  Doing that will require a little more code restructuring.  The inner loop can simply be this:

input[c1].addNeighbor(input[c2]);

And rewrite the member function like this:

void City::addNeighbor(City &city) {
    if (getCityID() == city.getCityID()) {
        return;
    }
    unsigned distance = Utilities::distanceSquared(*this, city);
    neighbors.emplace_back(Neighbor{city.getCityID(), distance});
    city.neighbors.emplace_back(Neighbor{getCityID(), distance});
}

Note that we avoid calculating the distance if they are the same ID, but not if the distance between them is actually zero.  In this case the argument to the function must not be const because we need also to modify it.

Also, although you did not provide the code for it, I notice that you have a NearestNeighbor() function.  There are some efficient ways to deal with that, depending on how you intend to use it.  One could sort the vectors (once) when population of them is complete or one might use a std::priority_queue instead of a std::vector.

Pass const references where practical

Passing an object by value can cause an object copy.  For example this function:

unsigned int Utilities::distanceSquared(City a, City b);

should instead be written like this:

unsigned int Utilities::distanceSquared(const City &a, const City &b);

Avoid floating point if only integers are needed

Integer operations are usually faster than floating point operations.  So here's how I'd write your distanceSquared function:

namespace Utilities {
unsigned int distanceSquared(const City &a, const City &b) {
    int dx{b.getXCoord() - a.getXCoord()};
    int dy{b.getYCoord() - a.getYCoord()};
    return dx*dx + dy*dy;
}
}

As @TobySpeight correctly points out, this assumes that no overflow occurs.  On my randomly generated test data, the ranges of the x and y coordinates are restricted so that is guaranteed, with my compiler, that no overflow can occur, but it's worth verifying that for your real data as well.


QSlider direct jump on click By default, QSlider move his thumbtrack by a value belonging to the singleStep() prop on mouse click. To make thumbtrack jump directly at the mouse click point, we need to create a new class inherited by QSlider.

Header file (.h):

#include <QWidget>
#include <QAbstractSlider>
#include <QSlider>
#include <QMouseEvent>
#include <QStyle>
#include <QStyleOptionSlider>
#ifndef QIMPROVEDSLIDER_H
#define QIMPROVEDSLIDER_H

class QImprovedSlider : public QSlider
{
    Q_OBJECT
protected:
    void mousePressEvent(QMouseEvent *event);

public:
    explicit QImprovedSlider(QWidget *parent = 0);

    ~QImprovedSlider();

public slots:

private:

private slots:

signals:
    void onClick(int value);
};

#endif // QIMPROVEDSLIDER_H

Source file (.cpp):

#include "QImprovedSlider.h"

QImprovedSlider::QImprovedSlider(QWidget *parent) :
    QSlider(parent)
{

}

QImprovedSlider::~QImprovedSlider()
{
}

void QImprovedSlider::mousePressEvent(QMouseEvent *event) {
  QStyleOptionSlider opt;
  initStyleOption(&opt);
  QRect sr = style()->subControlRect(QStyle::CC_Slider, &opt, QStyle::SC_SliderHandle, this);

  if (event->button() == Qt::LeftButton &&
      !sr.contains(event->pos())) {
    int newVal;
    if (orientation() == Qt::Vertical) {
       double halfHandleHeight = (0.5 * sr.height()) + 0.5;
       int adaptedPosY = height() - event->y();
       if ( adaptedPosY < halfHandleHeight )
             adaptedPosY = halfHandleHeight;
       if ( adaptedPosY > height() - halfHandleHeight )
             adaptedPosY = height() - halfHandleHeight;
       double newHeight = (height() - halfHandleHeight) - halfHandleHeight;
       double normalizedPosition = (adaptedPosY - halfHandleHeight)  / newHeight ;

       newVal = minimum() + (maximum()-minimum()) * normalizedPosition;
    } else {
        double halfHandleWidth = (0.5 * sr.width()) + 0.5;
        int adaptedPosX = event->x();
        if ( adaptedPosX < halfHandleWidth )
              adaptedPosX = halfHandleWidth;
        if ( adaptedPosX > width() - halfHandleWidth )
              adaptedPosX = width() - halfHandleWidth;
        double newWidth = (width() - halfHandleWidth) - halfHandleWidth;
        double normalizedPosition = (adaptedPosX - halfHandleWidth)  / newWidth ;

        newVal = minimum() + ((maximum()-minimum()) * normalizedPosition);
    }

    if (invertedAppearance())
        this->setValue( maximum() - newVal );
    else
        this->setValue(newVal);

    event->accept();
  }
  else {
        QSlider::mousePressEvent(event);
  }
  emit onClick(this->value());
}

This code works well, so is not a broken code. There is a way to improve this? Maybe trying to differentiate the direct jump from the handle dragging

if (invertedAppearance()) {
        this->setValue( maximum() - newVal );
} else {
        this->setValue(newVal);
}

You should not be posting if/else statements without Curly Braces. The style leaves you open for massive bugs, and as an after thought: makes it cumbersome for you or other developers to add to this code.

So for example, let's say you are editing or debugging your code, and decide to comment out a line:

if (invertedAppearance())
    this->setValue( maximum() - newVal );
else
    // this->setValue(newVal);

event->accept();

Do you see the issue?


Expect all values in an array to be greater than 30 I've given an array of elements and need to check that every single text is a number greater than 30. Sample input data:

40

31

32

101

Currently, I'm using .each() function to process every element one by one and then parseInt() and toBeGreaterThan() jasmine matcher:

var dataItems = element.all(by.css("ul.data-list > li.data-item"));
dataItems.each(function (elm) {
     elm.getText().then(function (text) {
         expect(parseInt(text)).toBeGreaterThan(30);
     });
});

Is there a simpler or better way to address the problem? Would it be appropriate to first map() into an array of integers and then apply the matcher to every element of an array?

What I am a bit concerned about in using the current approach is that it would fail at the very first element that is less than or equal to 30 preventing us to see what other values of an array are.

Array::some can be used to reverse the test logic and return earlier.


Finding pair in an array with the OO approach Description:

I am trying to solve algorithmic problems mixing them with Object oriented design as part of my interview preparation. So, the question is given an array of integers find the pair of index which satisfy the given sum hence, given:

  arr = [1, 2, 3, 4, 5] , sum = 6, answer = (2, 4) or (1, 5)

Code:

class Solution {
    static final class Pair {
        public final int i;
        public final int j;

        public Pair() {
            i = -1;
            j = -1;
        }

        public Pair(int i, int j) {
            this.i = i;
            this.j = j;
        }

        public String toString() {
            StringBuilder sb = new StringBuilder();
            return "(" + i + ", " + j + ")";
        }
    }

    static final class PairSum {
        private Pair p;

        public PairSum(int[] arr, int sum) {
            // validation here
            int n = arr.length;
            boolean found = false;
            for (int i = 0; i < n; i++) {
                if (found == true) break;
                for (int j = i + 1; j < n; j++) {
                    if (arr[i] + arr[j] == sum) {
                        p = new Pair(i, j);
                        found = true;
                    }
                }
            }
            if (p == null) // no pair found
                p = new Pair();
        }

        public Pair get() {
            return p;
        }
    }

    static final class PairSumFast {
        private Pair p;

        public PairSumFast(int[] arr, int sum) {
            // validation here
            Map<Integer, Integer> map = new HashMap<>();

            for (int i = 0; i < arr.length; i++) {
                int diff = sum - arr[i];
                if (map.containsKey(diff)) {
                    p = new Pair(map.get(diff), i);
                    break;
                }
                else
                    map.put(arr[i], i);
            }
            if (p == null) p = new Pair();
        }

        public Pair get() {
            return p;
        }
    }

    public static void main (String[] args) throws java.lang.Exception {
        PairSum p = new PairSum(new int[]{1, 2, 3, 0, 4}, 4);
        System.out.println(p.get()); // (0, 2)

        p = new PairSum(new int[]{1, 2, 3, 0, 4}, 5);
        System.out.println(p.get()); // (0, 4)

        p = new PairSum(new int[]{1, 2, 3, 0, 4}, 10);
        System.out.println(p.get()); // (-1, -1)

        PairSumFast p2 = new PairSumFast(new int[]{1, 2, 3, 0, 4}, 4);
        System.out.println(p2.get()); // (0, 2)

        p2 = new PairSumFast(new int[]{1, 2, 3, 0, 4}, 5);
        System.out.println(p2.get()); // (0, 4)

        p2 = new PairSumFast(new int[]{1, 2, 3, 0, 4}, 10);
        System.out.println(p2.get()); // (-1, -1)
    }
}

Given the time constraint I am not sure if its a good approach but I think it should a good impression. I would like to hear if my code could be improved further?

The biggest problem here IMO is that, from a OOP point of view, it's a bit difficult to represent an algorithm as an object. Objects usually represent "things" which have methods that represent "actions" that happen to those things. Algorithms are usually used in such actions. The object in this case would be the array, but since in Java arrays aren't objects and Java doesn't have the concept of extension methods, it's difficult to represent a stand-alone algorithm in an OOP way.
EDIT: I just realized, your algorithms are returning a pair of indices instead of values from the arrays. That doesn't correspond to the example, and in most cases the pair of values would be more useful than the indices. Also my remarks are based on the assumption that values are returned.
Never-the-less here are some remarks about the OOP side of things:
class Pair
The biggest problem here is the usage of -1 to represent ... what exactly? Keep in mind -1 is  a completely "normal" valid integer. A developer who uses that class and who doesn't explicitly check for that -1 will get no error message when using it in calculations and instead his program most likely will run fine, but display wrong results. More importantly your program currently displays "(-1, -1)" when there is no solution, but it looks like a possible solution. What if the array contains two -1s and you ask for a sum of -2? How do you distinguish between a correct result and no result.
If you actually should need such an "empty" value of Pair you should use Integers and null. However in your case there is no need for such an empty result. Just drop the default constructor. Instead just have your algorithm return null or - if you don't like using null - use Java 8's Optional.
Pair is a perfect case for an immutable object. There is no reason to change one of the values in the pair after creation. Make the fields private and create getters for them.

PairSum/PairSumFast
Since both classes do the same thing, it would be worth considering have them implement a common interface. For such a small example it may be over-engineered, but it would display some insight into OOP.
Another thing that would show a bit more OOP would be to use the Collection interface instead of an array to provide the input. In Java it is rare to actually use an array as a "top level" data structure, since they are not objects are inflexble and don't fit in with Java's collection infrastructure. Arrays are usually used as a low level data storage structure and hidden in the implementation details of classes.


Sorting an array of numbers in Java with an algorithm This is pretty much one of my first programs that I have created in Java and I just wanted to ask if anyone sees some obvious errors or mistakes I made.

The purpose of this program is to sort numbers given by a user using an algorithm.

First off, the program gets an input in form of ints from the user, I have used a popup here, and than creates an array arr1 with that size.
After that, another popup opens that asks the user for the 1st number he wants to sort. Then the 2nd, 3rd, etc, until the array has been filled with ints.

The array gets transferred into the sorting algorithm which, surprise, sorts the numbers into a different array arr2.

This array then gets passed into a method that takes a static frame and puts a text field with the numbers (in correct order) onto it. [+ 1 close and 1 repeat button]

If the repeat method (buttonRetry.addActionListener.actionPerformed) is called / the button is pressed, the frame gets "cleaned" (main_frame.getContentPane().removeAll();) and the main() method is called again.

Is it possible to make the process of this happening (getting input from the user and sorting that input) faster and do you have any additional advice about what and how I did this (Noob errors, graphical tips, etc.).

Here is the code :

import java.util.*;
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.awt.Color;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;

public class AWTCounter extends JFrame {

    public static int[] arr1;
    public static JFrame frame = new JFrame("Super Simple Sorting Program");
    public static JFrame main_frame = new JFrame("Super Simple Sorting Program");

    public void main()
    {
        String input_1 = JOptionPane.showInputDialog(frame, "How many numers do you want to sort with SSSP ?");
            if (input_1 == null) {dispose(); System.exit(0);}
        int checksum = Integer.parseInt(input_1);

        arr1 = new int[checksum];

        int logNumber = 0;
        int debug = checksum;
        checksum = 0;

        while (checksum <= debug-1)
        {
            logNumber++;
            String input_2 = JOptionPane.showInputDialog(frame, "Please enter your " + logNumber + " . number.");
            int member = Integer.parseInt(input_2);
            arr1 [checksum] = member ;
            checksum++;
        }

        int[] arr2 = doSelect(arr1);

        print(arr2);
    }

    public int[] doSelect(int[] arr)
    {

        for (int i = 0; i < arr.length - 1; i++)
        {
            int index = i;
            for (int j = i + 1; j < arr.length; j++)
                if (arr[j] < arr[index])
                    index = j;

            int smallerNumber = arr[index];
            arr[index] = arr[i];
            arr[i] = smallerNumber;
        }
        return arr;
    }

    public void print(int[] arr2)
    {
        JTextField t2;

        t2 = new JTextField("Here are your sorted numbers : " + Arrays.toString(arr2));
        main_frame.add(t2, BorderLayout.NORTH);

        JButton buttonEnd = new JButton("Close apllication");
        main_frame.add(buttonEnd, BorderLayout.CENTER);

        JButton buttonRetry = new JButton("Try again !");
        main_frame.add(buttonRetry, BorderLayout.SOUTH);

        main_frame.setLayout(new FlowLayout());
        main_frame.setSize(600,300);
        main_frame.setVisible(true);

        buttonEnd.addActionListener(new ActionListener()
        {

            public void actionPerformed(ActionEvent event)
            {
                dispose();
                System.exit(0);
            }

        });

        buttonRetry.addActionListener(new ActionListener()
        {

            public void actionPerformed(ActionEvent event)
            {
                main_frame.getContentPane().removeAll();
                main();
            }

        });

    }
}

Im impressed that you used selection sort. The standard bubble sort method would be slower. Have a look at the bubble sort method anyway.

int n = arr.length, r;
for (int i = 0; i < n; i++)
{
    for (int (j = i + 1); j < n; j++)
    {
        if (arr[i] > arr[j])
        {
            r = arr[i];
            arr[i] = arr[j];
            arr[j] = r;
        }
    }
}

You could also have used the default Arrays.sort method

Arrays.sort(arr);

You are creating an applet. In this case, it would probably be good to declare the main method as static

When you are importing the packages, why have you imported the individual classes after importing them by wildcard character '*'; There is no need for it.
That is redundant and the computer wastes time re-importing and garbage-collecting. Just do:

import java.util.*;
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

Also, printing mistake here:

String input_1 = JOptionPane.showInputDialog(frame, "How many numbers do you want to sort with SSSP ?");

Consider setting bounds to the buttons and text boxes in your mainframes, to make the locations more exact. That way, it will be easier to add a new GUI component to the window when you want to add it.
The command goes:
<name of object>.setBounds(//x-axis from left, //y-axis from top, //width, //height)
For example:

t2 = new JTextField("Here are your sorted numbers : " + Arrays.toString(arr2));
t2.setBounds(20, 40, 75, 10);
main_frame.add(t2);

JButton buttonEnd = new JButton("Close application");
t2.setBounds(95, 40, 75, 10);
main_frame.add(buttonEnd);

JButton buttonRetry = new JButton("Try again !");
t2.setBounds(180, 40, 75, 10);
main_frame.add(buttonRetry);

WHY have you named your class AWTCounter? That makes me think you are counting numbers with certain properties or words in a sentence. Won't AWTSorter be better?

Great program though, especially if it's your first!


Geocoder form with Google Maps autosuggest Even though it's simple, I've spent quite a bit of time on it, learning as I go. Is there anyway I can make this code more efficient, more crossbrowser compatible or just generally better?

jsFiddle

<!-- Geocode -->
$(document).ready(function () {
    var GeoCoded = { done: false };
    autosuggest();
    $('#myform').on('submit', function (e) {
    if (GeoCoded.done) return true;

        e.preventDefault();
        var geocoder = new google.maps.Geocoder();
        var address = document.getElementById('location').value;

        $('#myform input[type="submit"]').attr('disabled', true);

        geocoder.geocode({
            'address': address
        },

        function (results, status) {
            if (status == google.maps.GeocoderStatus.OK) {
                var latLng = results[0].geometry.location;
                $('#lat').val(results[0].geometry.location.lat());
                $('#lng').val(results[0].geometry.location.lng());
                GeoCoded.done = true;
                $('#myform').submit();
            } else {
                console.log("Geocode failed: " + status);
                //enable the submit button
                $('#myform input[type="submit"]').attr('disabled', false);
            }

        });

    });

});

function autosuggest() {
        var input = document.getElementById('location');
        var options = {
            types: [],
        };
        var autocomplete = new google.maps.places.Autocomplete(input, options);
    }

From a once over:

When you wrote

var latLng = results[0].geometry.location;
$('#lat').val(results[0].geometry.location.lat());
$('#lng').val(results[0].geometry.location.lng());

you probably meant to write

var latLng = results[0].geometry.location;
$('#lat').val(latLng .lat());
$('#lng').val(latLng .lng());

I wonder why you capture new google.maps.places.Autocomplete(input, options); into autocomplete since you do nothing with autocomplete.
1 var block with comma separated variables is considered the standard, preferably on top.
You could capture document.getElementById('location') into a var to prevent repeated DOM lookups
Don't do console.log in production code
Creating GeoCoded as an object with 1 property is overkill, why not simply
var geoCodingDone = false;
lowerCamelCasting is not applied consistently : autosuggest -> autoSuggest, autocomplete -> autoComplete
There is an indentation problem after return true;

With all that, I would counter suggest something like

$(document).ready(function () {

  var geoCodingDone = false,
      locationInput = document.getElementById('location');

  createAutoSuggest();

  $('#myform').on('submit', function (e) {
    if (geoCodingDone)
      return true;

    var geocoder = new google.maps.Geocoder(),
        address  = locationInput.value;

    e.preventDefault();

    $('#myform input[type="submit"]').attr('disabled', true);

    geocoder.geocode( {'address': address }, function (results, status){
        if (status == google.maps.GeocoderStatus.OK) {
            var latLng = results[0].geometry.location;
            $('#lat').val(latLng.lat());
            $('#lng').val(latLng.lng());
            geoCodingDone = true;
            $('#myform').submit();
        } else {
            $('#myform input[type="submit"]').attr('disabled', false);
        }
    });
  });
});

function createAutoSuggest(){
  var options = { types: [] };
  new google.maps.places.Autocomplete(locationInput, options);
}