Improving rendering performance of 2D Tile Game I want to see if I can make any improvements to the rendering methods that are currently written. I've noticed when profiling this project that the CPU was allocating a fair percentage of the time towards rendering and was wondering if there were any improvements that I can make towards it.

Note: Below is my somewhat simple rendering method. I am rendering an ArrayList of tiles, few of these are transparent. This function (y*map.getMapWidth())+x) will return the index in the ArrayList to be rendered.

ImageManager is a class that stores cropped images from a spritesheet as a BufferedImage.

import java.awt.Canvas;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.image.BufferStrategy;
import java.awt.image.BufferedImage;

public class Game extends Canvas implements Runnable{

    private void renderTiles(Graphics g) {
        // the overlay variable determines if I need to render anything off screen
        // (True if player is moving)
        int overlay = 0;
        if (moving) {
            overlay = 1;
        }

        // the four variables below determine the starting/finishing x/y variables
        // to begin rendering
        int startX = -((xOffset/(Tile.TILESIZE*SCALE))+overlay);
        int finishX = -(((xOffset-(WIDTH*SCALE))/(Tile.TILESIZE*SCALE))-overlay);
        int startY = -((yOffset/(Tile.TILESIZE*SCALE))+overlay);
        int finishY = -(((yOffset-(HEIGHT*SCALE))/(Tile.TILESIZE*SCALE))-overlay);

        // to avoid crashes, the variables can't render what isn't there in the list
        if (startX < 0) {
            startX = 0;
        } if (finishX > map.getMapWidth()) {
            finishX = map.getMapWidth();
        } if (startY < 0) {
            startY = 0;
        } if (finishY > map.getMapHeight()) {
            finishY = map.getMapHeight();
        }

        // where the tiles are render. tiles are stored in an ArrayList so that
        // switching between maps is easier
        for (int y = startY; y < finishY; y++) {
            for (int x = startX; x < finishX; x++) {
                map.tile.get((y*map.getMapWidth())+x).render(g);
            }
        }
    }

    public void render(){
        BufferStrategy bs = this.getBufferStrategy();
        if(bs ==  null){
            createBufferStrategy(2);
            return;
        }
        Graphics g = bs.getDrawGraphics();
        // start rendering

        g.fillRect(0, 0, WIDTH * SCALE, HEIGHT * SCALE);
        renderTiles(g);
        player.render(g);
        g.dispose();
        bs.show();

    }
}

import java.awt.Graphics;
import java.awt.image.BufferedImage;

public abstract class Tile {

    public static final int TILESIZE = 16;

    protected BufferedImage bi;
    protected ImageManager im;
    protected int x, y, oX, oY;
    protected Game game;

    public Tile(int x, int y, ImageManager im, Game game, BufferedImage bi){
        this.oX = x;
        this.oY = y;
        this.im = im;
        this.game = game;
        this.bi = bi;
    }

    public abstract void tick();
    public abstract void render(Graphics g);

}

import java.awt.Graphics;
import java.awt.image.BufferedImage;

public class WalkableTerrainTile extends Tile{

    public WalkableTerrainTile(int x, int y, ImageManager im, Game game, BufferedImage bi) {
        super(x, y, im, game, bi);
    }

    public void tick() {
        this.game  = game;
        x = oX + game.xOffset;
        y = oY + game.yOffset;
    }

    public void render(Graphics g) {
        g.drawImage(bi, x, y, Tile.TILESIZE * Game.SCALE, Tile.TILESIZE * Game.SCALE, null);
    }

}

I'm not sure if these would be optimized out by the compiler but

Tile.TILESIZE * SCALE
Tile.TILESIZE * Game.SCALE

are done more than once, why not just keep them as variables and...

for (int y = startY; y < finishY; y++) {
 for (int x = startX; x < finishX; x++) {
   map.tile.get((y*map.getMapWidth())+x).render(g);

...why do a multiplication for every y? you could just have a variable and increment it

int yLoc = startY * map.getWidth();
for (int y = startY; y < finishY; y++) {
 for (int x = startX; x < finishX; x++) {
   map.tile.get(yLoc+x).render(g);
 }
 yLoc += map.getMapWidth();
}

then you only need to do one multiplication and one addition for each y step instead of a multiplication for every y


Using something other than form field's value in Vue I have a form, I need to send untraslated values, but as well, I need to get translated ones from Vue to write them down in page.

For now, I ended up creating a 'shadow' property and changing it via the watch function. Note that I'm referencing the SELECT via the custom ref prop and Vue.$refs. So:

<select name="screw[type]" ref="screw_type_select" v-model="form.screw.type">
   <option value="My value" data-value="<?php _e('My value', 'context'); ?>"><?php _e('My value', 'context'); ?></option>
   //[...]

Then in Vue:

var vueapp = new Vue({
  el: '#form'
  ,data:{
    form:{
      ,screw:{
        type: "Svasata Piana"
        ,type_t: "Svasata Piana"
      }
    }
  }// data
  ,watch:{
    'form.screw.type':function(){
      var select = this.$refs.screw_type_select;
      this.form.screw.type_t = select.options[select.selectedIndex].getAttribute('data-value')
    }
  }
});

Then again in html:

{{ form.screw.type_t }} // instead of {{ form.screw.type }}

So the form will send value, but the user will see data-value intead (which is translated).

What do you think of this way?

This is not the Vue-way to do it. Vue does not recommend the use of $refs. And in this case, you can also avoid it.

Your current code is:

<select name="screw[type]" ref="screw_type_select" v-model="form.screw.type">
   <option value="My value" data-value=""></option>

I would recommend using v-for to loop through your options, and keep all your options in your data.

So for example: (This is how it is done in the Vue JS official documentation)

data() {
    return {
        options: [
             {
                 text: "<?php _e('My value', 'context'); ?>",
                 value: "My value"
             },
             ...
        ]
    }
}

And then:

<select name="screw[type]" v-model="form.screw.type">
  <option v-for="option in options" v-bind:value="option.value">
    {{ option.text }}
  </option>
</select>

Instead of using the watch, you can use a computed property.

computed: {
    selectedScrewType() {
        return this.options[this.form.screw.type].text;
    }
}

Then you can use {{ selectedScrewType }} in your template.


Tiny JavaScript live filter I'm looking for feedback on this live-filter I wrote in JavaScript. You can test it using the included snippet.

(function() {
  var searchField = document.getElementById('filter');

  var handler = function() {
    var text = searchField.value,
      nodes = document.getElementsByClassName('phase'),
      regexp = new RegExp(text, 'i');

    for (var i = 0; i < nodes.length; i++) {
      var node = nodes[i];

      node.text.search(regexp) < 0 ?
        node.parentNode.style.display = 'none' :
        node.parentNode.style.display = 'block';
    }
  };

  searchField.onkeyup = handler;
  searchField.onchange = handler;
})();
<input type="text" id="filter" />

Ruby on Rails

The Rails Way

Pickaxe Guide

jQuery HTML

CSS HTML AJAX

HTML5 Net Set

Node Easy

Listing Bloop

So, a few notes:

Your filtering is too broad. If you had other elements in the DOM with class phase, they would also be filtered, even though they aren't part of the list. Instead, it's better to limit what you're selecting -- DOM elements have querySelectorAll, getElementsBy..., etc., to ensure that you're limiting the results to that element's children.
This can't be re-used very well -- make it more modular. You can do this by passing in the ID of the search field and the list (and any additional selectors).
I'd suggest using classes to toggle visibility, because at some point you might actually want the styling to be different. Note that classList is only available on modern browsers (http://caniuse.com/#feat=classlist).
Cache your lengths when iterating over an Array-like.
There's no need for parentNode if you use textContent. Then you can query for all lis in the list and filter them instead. This also means you don't have to add classes for each list item unless you want to. In my example, I've removed the class='phase' portion of your anchors.
I assume your items are eventually going to be links to elsewhere, so I left the anchors in the example.

var liveFilter = (function() {
  /**
   * Initializes a search field with associated list and filters it live while typing
   *
   * @param {string} searchFieldId     An ID of a search field input element
   * @param {string} listId            An ID of an element containing items that should be filtered
   * @param {object} [options]         Specifies additional options:
   *                                   * selector: specifies a query to be run against list; the results
   *                                     will be filtered according to the value of the search field. If
   *                                     not specified, the filter matches all direct children.
   *                                   * filterClass: specifies a class name to be applied to items that
   *                                     are going to be filtered out. If not specified, defaults to
   *                                     "filter-hidden".
   */
  function liveFilter( searchFieldId, listId, options) {

    var searchField = document.getElementById(searchFieldId),
        list = document.getElementById(listId),
        selector,
        filterClass;

    if (!searchField) {
      throw new Error("No search element found with id " + searchFieldId);
    }

    if (!list) {
      throw new Error("No list element found with id " + listId);
    }

    if (options !== undefined) {
      selector = options.selector;
      filterClass = options.filterClass;
    }

    if (filterClass === undefined) {
      filterClass = "filter-hidden";
    }

    var handler = function searchFieldChanged() {
      var text = this.value,     // this will be bound to the search field
          regexp = RegExp(text, 'i'),
          nodes = (selector === undefined) ? list.children
                                           : list.querySelectorAll(selector),
          node;
      for (var i = 0, l = nodes.length; i < l; i++) {
        node = nodes[i];
        if (node.textContent.search(regexp) < 0) {
          // note: classList is only available on modern browsers
          node.classList.add(filterClass);
        } else {
          node.classList.remove(filterClass);
        }
      }
    };

    // input is for modern browsers
    searchField.addEventListener("input", handler, false);
  }

  // If using CommonJS, you'd do module.exports = liveFilter, and it could be required later.
  return liveFilter;
})();

liveFilter("filter", "list", { selector: "li", filterClass: "filter-hidden" });
.filter-hidden {
  display: none;
}
<input type="text" id="filter" />

Ruby on Rails

The Rails Way

Pickaxe Guide

jQuery HTML

CSS HTML AJAX

HTML5 Net Set

Node Easy

Listing Bloop


Euler Project 14 Longest Collatz Sequence I have been working through the euler project and I have created the following code for Problem 14, which asks which starting number, under 1 million, produces the longest Collatz Sequence chain.

It takes a good minute to process.  Can I make it faster?

class Collatz:

    def __init__(self):
        self.c = 1

    def if_even(self, n):
        return int(n / 2)

    def if_odd(self, n):
        return int(n * 3 + 1)

    def decide(self, n):
        if (n & 1) == 0:
            return self.if_even(n)
        elif (n & 1) == 1:
            return self.if_odd(n)

    def iter_nums(self, n):
        if n == 1:
            return n
        self.c += 1
        n = self.decide(n)
        self.iter_nums(n)

largest = 0

# col = Collatz()
# col.iter_nums(13)

# print(col.c)

for i in range(1000000, -1, -1):
    col = Collatz()
    col.iter_nums(i)
    if col.c > largest:
        largest = i

print(largest)

Useless calls to int

When n is an integer:

n * 3 + 1 is an integer as well, you don't need to call int
int(n / 2) can also be computed using the "floored quotient" with the // operator.

You don't need a class

I highly recommend watching Jack Diederich's talk called "Stop Writing Classes". Classes are a great tool but they are not always needed, sometimes the problem can be solved with simple tools like functions.

In your case, the whole class can be restricted to a simple function, shorter, clearer, more efficient, easier to use.

def get_collatz_length(n):
    if n == 1:
        return 1
    return 1 + get_collatz_length((3 * n + 1) if (n & 1) else n // 2)

You don't need recursion

Once again, recursion can be a great way to write a concise solution for a given problem. However, in Python, it is sometimes best to avoid it: there is no tail-recursion optimisation and the call stack has a fairly limited size.

You could write something like:

def get_collatz_length_loop(n):
    l = 1
    while n != 1:
        n = (3 * n + 1) if (n & 1) else n // 2
        l += 1
    return l

To be continued: memoization

Is bitwise optimisation relevant?


Detecting a loop in a Linked list using a hashset I was wondering if detecting a loop in a linked list using a HashSet is better than using two pointers?

        HashSet<Integer> set = new HashSet<Integer>();
    LinkedListNode current = head;
    while(current != null) {
        if(current.next != null) {
            if(set.contains(current.next.data)) {
                return current.next; // this has been seen so there is a loop.
            }
            set.add(current.next.data);
        }
        current = current.next;
    }
    return null;

I think instead of adding integers to the Hashset, what we should do is insert a LinkedListNode instance into the Hashset and then to detect a loop we check whether the current node exists in the Hashset or not, if it does then there's a loop

 HashSet<LinkedListNode> set = new HashSet<LinkedListNode>();
    LinkedListNode current = head;
    while(current != null) {
        if(current.next != null) {
            if(set.contains(current.next)) {
                return current.next; // this has been seen so there is a loop.
            }
            set.add(current.next);
        }
        current = current.next;
    }
    return null;


Check if all of the delimiters in an expression are matched and closed Problem

Write an algorithm to determine if all of the delimiters in an
expression are matched and closed.
{(abc)22}[14(xyz)2] should pass
[ { ] } should fail
{ (x) } [ should fail

Any advice of code functional bug, performance in terms of algorithm time complexity, code style are appreciated.
delimiters = ['[', ']', '(', ')', '{', '}']
delimiters_map = {']':'[', ')':'(', '}':'{'}

def check_match(source):
    working_set = []
    for i,v in enumerate(source):
        if v not in delimiters:
            continue
        if v in delimiters_map: # end of a delimiter:
            if working_set[-1] != delimiters_map[v]:
                return False
            else:
                working_set.pop(-1)
        elif v in delimiters:
            working_set.append(v)

    if len(working_set) > 0:
        return False
    else:
        return True

if __name__ == "__main__":
    print check_match('{(abc)22}[14(xyz)2]')
    print check_match('[ { ] }')
    print check_match('{ (x) } [')

Performance

Checking if a list contains a value is an \$O(n)\$ operation.
To make if v not in delimiters fast,
it would be better to make delimiters a set instead of a list.

The elif v in delimiters: is unnecessary, because at that point we already know that v is in delimiters, so you can replace with a simple else:.

Creating lists and maps of characters

This is a PITA to type:

delimiters = ['[', ']', '(', ')', '{', '}']
delimiters_map = {']':'[', ')':'(', '}':'{'}

An easier and less error-prone way is to call list to turn a string into a list of characters:

openers = list('[({')
closers = list('])}')
delimiters = set(openers + closers)
delimiters_map = dict(zip(closers, openers))

If it's a stack, call it a stack

working_set is a misleading name. It suggests a set, but it's a list.
And you're using it as a stack, so I suggest to call it a stack.

Use doctests

Doctests are awesome. You can run them with python -mdoctests script.py.

def check_match(source):
    """
    >>> check_match('{(abc)22}[14(xyz)2]')
    True

    >>> check_match('[ { ] }')
    False

    >>> check_match('{ (x) } [')
    False

    >>> check_match('')
    True

    """
    # ... (your implementation)


Slider module and extending it further I created this module, and I'm wondering if I did a good job. How would an experienced JavaScript developer improve this simple slider module further? Like, let's say that you want to use this module on 3 sliders on the same page.

Here is a jsFiddle.

Module

var sliderTest = (function(){
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   private variables
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/
    var expoElement = document.querySelector(".test")
    var expoCanvas = expoElement.querySelector(".canvas")
    var expoSlides = expoCanvas.querySelectorAll(".slide")
    var controlPrev = expoElement.querySelector(".control.prev")
    var controlNext = expoElement.querySelector(".control.next")
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   private functions
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/
    function whereIsActive(){
       var y = {}
       for(var i = 0; i < expoSlides.length; i++){
          if(expoSlides[i].classList.contains("active")){
              y.current = i;
              if(expoSlides[i+1]){
                  y.next = i+1;
              } else {
                  y.next = 0;
              }

              if(expoSlides[i-1]){
                  y.prev = i-1;
              } else {
                  y.prev = expoSlides.length-1;
              }
          }
      }
      return y
   }

   function removeActiveClasses(){
       for(var i = 0; i < expoSlides.length; i++){
           expoSlides[i].classList.remove("active")
       }
   }
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   public methodes
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

        var publicMethod = {
            nextSlide:function(ev){
               ev.preventDefault();
               var nextIndex = whereIsActive().next;
               removeActiveClasses();
               expoSlides[nextIndex].classList.add("active");
       },

       prevSlide:function(ev){
           ev.preventDefault();
           var prevIndex = whereIsActive().prev;

           removeActiveClasses();
           expoSlides[prevIndex].classList.add("active")
      },
      sendInTheClone:function(){alert(whereIsActive().current)}
  }
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   event listners
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

            controlNext.addEventListener("click", publicMethod.nextSlide)
            controlPrev.addEventListener("click", publicMethod.prevSlide)
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   return
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

        return publicMethod;
})();

HTML

1

2

3

4

    prev next

document.querySelector
While it is a great function to use, you are using in the wrong situations.
For every time you use it, you are getting an array of elements with a class that you passed in to the function. Why not just use document.getElementsByClassName?
Sure, it takes a little bit longer to type but it's faster.
This about it: the querySelector function has to read the argument passed in and, based on it's syntax ('.' for classes, '#' for ids, and a few more), scan the document in a way unique to each type of identification (id, class, etc).
Wouldn't it be easier for the function to know that it was going to look for one class and for one class only?
Semicolons
They aren't entirely necessary, and the interpreter probably adds them in for you, but your code looks like JavaScript when you put them in (at least in my opinion).
HTML indentation
Your divs are perfectly indented, but when you go down to your as, you start to try and clump them on to one line.
Originally, when I was looking at the a tag(s), I thought it was one really long a tag.
JavaScript indentation
For the most part, it's really good. But there is one section that is bothering me: your public methods section.
I actually had to post this in to JSFiddle and click "Tidy up" to understand what was going on there.
I believe the problem is the closing bracket of nextSlide; it is too far back, and it looks like it is closing publicMethod.
Built in output
In the sendInTheClone method of publicMethod, it uses the function alert.
Not everyone using this module may want an alert popping up on their screen; maybe they want it written to the DOM, or maybe they use custom pop-up boxes.
(Excuse this section ("Built in output") if I misunderstood your sendInTheClone function)
Good things:

Contrary to what people have been commenting, I think that the big comments going across your code to split it up is a good idea. In my opinion, it helps with legibility.

The use of publicMethod. It's a pretty good idea; I like it.


python password hashing I simply made this for practice. But I want to know if it can be used in the realworld or not. Please point out any bad programming practices and how I can change them and look out for them next time.
Your advice or tip will be the invaluable rain
on my deadly isolated programming land.

import os
import hashlib
from base64 import b64encode, b64decode

def convert_to_bytes(value, value_name):
    if isinstance(value, str):
        return value.encode('UTF-8')
    else:
        raise ValueError('%s must be string or bytes' % value_name)

def derive_passhash(password, hash_name='sha512', salt=None, iterations=100000):
    '''This function returns str type password_hash.
    hash contains hash_name, iterations, salt, derived_key
    -> [hash_name] $ [iterations] $$ [salt] $s_k$ [derived_key] '''
    if salt is None:
        salt = os.urandom(32)
    if not isinstance(salt, bytes):
        salt = convert_to_bytes(salt, 'salt')
    if not isinstance(password, bytes):
        password = convert_to_bytes(password, 'password')
    if not isinstance(iterations, int):
        if isinstance(iterations, str) and iterations.isnumeric():
            iterations = int(iterations)
        else:
            print('iterations must be integer.')
            raise ValueError
    # derive passhash
    try:
        d_key = hashlib.pbkdf2_hmac(hash_name, password, salt, iterations)
        del password    # maybe it can help to make more secure?
    except ValueError as error:
        print('[!] Error cused %s' % error)
        # log(error)

    # put hash_name, salt, iterations and derived key together
    # and encode to base64 format.
    pre_v = '$'.join((hash_name, str(iterations))).encode('utf-8')
    end_v = b'$s_k$'.join((salt, d_key))
    total_value = b'$$'.join((pre_v, end_v))

    user_hash = b64encode(total_value).decode(encoding='utf-8')
    return user_hash

def check_password(user_hash, asserted_pw):
    if not isinstance(asserted_pw, bytes):
        asserted_pw = convert_to_bytes(asserted_pw, 'password')

    user_hash = b64decode(user_hash.encode('utf-8'))

    h_i, s_k = user_hash.split(b'$$')
    hash_name, iterations = h_i.decode('utf-8').split('$')
    salt, key = s_k.split(b'$s_k$')

    try:
        asserted_key = hashlib.pbkdf2_hmac(hash_name, asserted_pw,
                                           salt, int(iterations))
        del asserted_pw
    except ValueError as error:
        print('[!] Error cused %s' % error)

    if key == asserted_key:
        return True
    else:
        return False

Typo

Error cused -> Error caused

Memory security

del password    # maybe it can help to make more secure?

It's not harmful, but it probably also won't help much. Your theory seems to be that if an outside attacker gains access to your program's memory (which is already instant doom), that calling del will have removed the plaintext password from memory thus reducing the impact of an attack. Have a read through an explanatory article like Python Garbage Collector, which says that

  you can use the del statement that removes a variable and its reference (not the object itself) [...] The del statement removes the references to our objects (i.e., decreases reference count by 1). After Python executes the del statement, our objects are no longer accessible from Python code. However, such objects are still sitting in the memory

del does not force garbage collection. To do that, you would in theory need to call gc.collect().

The most that del would immediately do is mitigate an attack on access to the locals dictionary. Again, if such an attack is possible, you have much bigger problems on your hands.

Boolean expressions

if key == asserted_key:
    return True
else:
    return False

can be

return key == asserted_key


Getting permutations of a set I'd like some feedback on my implementation to get all the permutations of a set.

Eg:

@Test
public void test()
{
    def data = [["A","B"],["A","B","C","D"],["D","E","F"]]
    def expected = toResultSet("ABD,ABE,ABF,ACD,ACE,ACF,ADE,ADF,BAD,BAE,BAF,BCD,BCE,BCF,BDE,BDF");
    def result = SetPermutations.getSetPermutations(data);
    assert result.containsAll(expected);
    assert result.size() == expected.size();
}

Note that:

The input is a collection of collections
Each collection may have a different number of items
Each item may appear in more than 1 collection (in the input) (ie., data[0] and data[1] both contain "A")
Each resulting permutation must contain exactly 1 item from each of the input sets
Each item may only appear once in a given permutation (ie., 'AAB' is invalid)

Here is my implementation:
(Note, my test is in groovy, but the implementation is java)

public class SetPermutations<E> {

    private List<E> permutationSet;
    private final Collection<Collection<E>> input;
    private final int maxDepth;
    private final Set<List<E>> result;

    public static <E> Set<List<E>> getSetPermutations(Collection<Collection<E>> input)
    {
        SetPermutations<E> setPermutations = new SetPermutations<E>(input);
        return setPermutations.build();
    }

    private SetPermutations(Collection<Collection<E>> input)
    {
        this.input = input;
        maxDepth = input.size() - 1;
        result = new HashSet<List<E>>();
    }

    private Set<List<E>> build() {
        List<Collection<E>> list = new ArrayList<Collection<E>>(input);

        int currentDepth = 0;
        permutationSet = newPermutationSet();

        permute(list,currentDepth);

        return result;
    }
    private void permute(List<Collection<E>> list,
            int currentDepth) {
        Collection<E> collection = list.get(currentDepth);
        for (E object : collection) {
            if (permutationSet.contains(object))
                continue;

            permutationSet.add(object);

            if (currentDepth != maxDepth)
            {
                permute(list, currentDepth + 1);
                permutationSet.remove(object);
            } else {
                result.add(permutationSet);
                // Allow the recursion to continue on a new permutationSet,
                // but remove our item from it.
                permutationSet = clonedPermutationSet();
                permutationSet.remove(object);
            }
        }
    }

    private List<E> newPermutationSet() {
        return new ArrayList<E>();
    }
    private List<E> clonedPermutationSet() {
        return new ArrayList<E>(permutationSet);
    }
}

I have a few specific questions:

Am I reinventing the wheel here?  I searched, and couldn't find a suitable implementation for this problem anywhere, but I find it hard to believe that this hasn't been solved before (better than my solution)
Is there a better way for me to have implemented this?

There's a number of things going on here, but the first thing that comes to mind is that you're placing an object on the heap that you simply discard (the entire class) for no apparent reason (speaking logically, not in terms of objects created).  Given that you don't appear to keep the object at all, this is unnecessary - do this all with static methods.  Additionally, this is not threadsafe, and the basic design cannot be adjusted to execute on multiple threads.  Furthermore, you're returning a Set<List<E>>, but your results are logically Set<Set<E>> (because you don't have repeats) - You probably ought to either adjust your results, or change your return results.

There's also a compile error in your code since you switched to the generics version, because of type inference (and this took me a while to figure out).

Most of the disconnect is occurring because of the odd blend of static and instance methods.  Additionally, you're attempting to use a builder pattern for something that doesn't really call for it - as near as the caller of your method knows, your SetPermutations class is never instantiated (which is false).  Also, you add object to permutationSet... and then almost immediately remove it.  Regardless of the performance implications of that, it's a terrible thing to do to a developer's mind.

Here's my (intial) take on the issue.  Note that this version is threadsafe (so long as T is an immutable object), and amenable to (easier) threading.  There are a few areas that could maybe be tweaked, especially as I'm creating a few more objects than I actually need (notably, my termination condition in permute), but this should be a better starting place.

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

public abstract class SetPermutations {

    private SetPermutations() {
        // no state.
    }

    public static <T> Set<Set<T>> getSetPermutations(final Collection<? extends Collection<T>> input) {
        if (input == null) {
            throw new IllegalArgumentException("Input not provided!");
        }
        final List<Set<T>> saved = new ArrayList<Set<T>>();
        for (Collection<T> c : input) {
            Set<T> s = new HashSet<T>(c);
            c.remove(null);
            if (c.size() >= 1) {
                saved.add(s);
            } else {
                throw new IllegalArgumentException("Input includes null/empty collection!");
            }
        }

        return permute(new HashSet<T>(), saved);
    }

    private static <T> Set<Set<T>> permute(final Set<T> initial, final List<Set<T>> itemSets) {

        if (itemSets.isEmpty()) {
            return Collections.singleton(initial);
        }

        final Set<T> items = itemSets.get(0);
        final List<Set<T>> remaining = itemSets.subList(1, itemSets.size());
      final int computedSetSize = initial.size() * items.size() * remaining.size();
        final Set<Set<T>> computed = new HashSet<Set<T>>(computedSetSize, 1);

        for (T item : items) {
            if (!initial.contains(item)) {
                Set<T> permutation = new HashSet<T>(initial);
                permutation.add(item);
                computed.addAll(permute(permutation, remaining));
            }
        }

        return computed;
    }

}

And here's the (revised) JUnit test to prove behaviour:

@Test
public void setPermutationsTest() {
    List<List<String>> data = new ArrayList<List<String>>();
    data.add(Arrays.asList("A", "B"));
    data.add(Arrays.asList("A","B","C","D"));
    data.add(Arrays.asList("D","E"));
    Set<Set<String>> expected = new HashSet<Set<String>>();
    expected.add(new HashSet<String>(Arrays.asList("A", "B", "D")));
    expected.add(new HashSet<String>(Arrays.asList("A", "B", "E")));
    expected.add(new HashSet<String>(Arrays.asList("A", "C", "D")));
    expected.add(new HashSet<String>(Arrays.asList("A", "C", "E")));
    expected.add(new HashSet<String>(Arrays.asList("A", "D", "E")));
    expected.add(new HashSet<String>(Arrays.asList("B", "C", "D")));
    expected.add(new HashSet<String>(Arrays.asList("B", "C", "E")));
    expected.add(new HashSet<String>(Arrays.asList("B", "D", "E")));
    Set<Set<String>> result = SetPermutations.<String>getSetPermutations(data);
    assertTrue(result.containsAll(expected));
    assertTrue(result.size() == expected.size());
}

(I removed "F" from the last collection, as it wasn't adding anything in terms of conditions - it fufilled the same function as "E").

EDIT:

The main reason why this is (more, and as much as can be reasonably coded for) threadsafe is the fact that you're not (completely) operating on a 'saved' copy:

You save off maxDepth before copying the 'exterior' collection.  If the size of that collection changes, maxDepth's value is now incorrect (results undefined).
You copy (and work from) the 'exterior' collection, but don't copy the 'interior' collections (The copy constructors might be deep for existing collections, but I doubt it, and won't be for custom collections).  This means that if any of the interior collections are modified during process run, you won't get correct results.

As for the static method instantiating a version of itself - there's no problem with that.  In fact, that generally forms the basis of a Factory Pattern.  The catch is that the instantiation is being returned - that is, the desired result is the instantiation.  Here, the only result is that you end up with a confusing use of instance variables.  The permutationSet methods end up being unwanted noise (It would have been better to explicitly just use the constructors), and are misleading to boot (you're returning a LIST).  Additionally, because of the use of the instance variables, it becomes non-trivial to reason about the state of the various collections.  NONE of the work done by instance methods benefits significantly from working on instance variables.  The only (theorhetical) benefit gained is that you work with a single copy of a result HashSet (because I define intermediate ones) - however, if it's an issue, the necessary collection should be passed in (as in a load() method.
Also, by permorming the instantiation as you have, you've completely eliminated any potential hope of threading the process (a potential concern, given that this is basically an O(n ^ m) process, where n is the number of collections, and m is the size of the collections, assuming roughly equal collection sizes).  With a thread pool and using futures, a non-instantiated version (or at least, not instantiated like this) can be threaded fairly trivially.


Fast nth prime in Python using trial division I'm trying to speed this code up for larger values of N. Any suggestions?
I'd like to stick to this implementation and not move on to something more complex like numpy or sieves.
Is there any low hanging performance I'm missing?

import time

start = time.clock()
def prime(num, divs):
    prev = 1
    if divs:
        for x in divs:
            if num % x == 0:
                return False
            if num/x < prev:
                break
            prev = x
    return True

i = 0
j = 2
k = []

while i  != 50000:
    if prime(j, k) == True:
        k.append(j)
        i += 1
        #if i % 50000 == 0:
        #   print (i)
    j += 1

print ("Done", j)
print ("Time", time.clock()-start)

time.sleep(50)

Division is a comparatively expensive operation, and it accounts for the lion's share of the time taken by trial division. Hence there are two major routes for improving efficiency:

reduce the cost of divisions by replacing them with multiplications
reduce the number of divisions by filtering candidates efficiently

Replacing divisions as a means of divisibility testing (i.e. x % p == 0) with multiplicatoin (x * MINV <= QMAX) gives a seven-fold speed increase in C++, but in Python it will be a lot less because of the interpreter overhead.

'Filtering candidates' means eliminating multiples of small primes during enumeration of the candidates that will be subjected to trial division. The most famous variant is skipping multiples of two (a.k.a. 'odds-only' or 'mod 2 wheel'), but also fairly well known - often under almost superstitious guises - is the mod 6 wheel that steps in the rhythm 4,2,4,2,... A further savings comes from the fact that the small primes skipped during enumeration can also be skipped during trial division. That is, when candidates are stepped mod 6 then the trial division would start with the prime 5, not 2.

Benchmarks, detailed explanations - and links to even more detailed explanations including papers - are in my answer to

Prime sieve implementation (using trial division) in C++

Code in that topic is in C++ and C#, but the principles and techniques apply to any language.


Generating a list of unique random longs I'm using the
Bogus for .NET: C#, F#, and VB.NET library.
What would be a more efficient way to generate a list of unique random longs?
public IEnumerable<long> GenerateRandomNumbers(int count = 1)
{
      var f = new Faker();
      List<long> temporaryList = new List<long>();

      for (int i = 0; i < 10000; i++)
      {
          var number = f.Random.Long(5000000000, 8000000000);
          temporaryList.Add(number );
      }

      return temporaryList.Distinct().Take(count).ToList();
}

I doubt that algorithmic efficiency can be improved but you may consider an implementation similar to this which leverages idiomatic LINQ.
public IEnumerable<long> GenerateRandomNumbers()
{
    var f = new Faker();

    return
        Enumerable.Range(0, int.MaxValue)
        .Select(
            _ => f.Random.Long(5000000000, 8000000000))
        .Distinct();
}


Extracting initials with R Playing around in R with a Edx CS50 homework. The job is meant to be done in C, I am using (the more familiar) R just for warmup.

I mainly wonder whether I am breaking any good practice/style norms overall or in any tiny detail.

initials <- function(full.name) {
  # Returns initials of a full name
  # Input will contain only letters (uppercase and/or lowercase) plus
  # single spaces between words. Folks like Joseph Gordon-Levitt,
  # Conan O’Brien, and David J. Malan won’t be using your program. (If only!)
  if (length(full.name) == 0) {
    stop ("Valid name please")
  } else {
    isspace  <- integer(0)
    fn.split <- unlist(strsplit(full.name, fixed = TRUE, split = ""))
    isspace  <- which(fn.split == " ")
    init     <- toupper(fn.split[c(1, (isspace+1))])
  return(paste(init, collapse = ""))
  }
}

initials("Stack Overflow")
[1] "SO"

You have a bug. length doesn't return the length of a string, but the length of a vector. The function you need is nchar.
This problem reveals that you haven't tested your program enough.
Always look for corner cases and test well.

A minor thing, but when you reach a stop statement, the rest of the function will not be executed, so you could remove the else block,
making the code more "flat", which is typically more readable.

As far as good practices go,
in R, it's generally not recommended to use return statements,
especially when there is a single point of return from the function.
So I recommend to simply omit the return statement at the end of the function.

Putting the above tips together:

initials <- function(full.name) {
  # Returns initials of a full name
  # Input will contain only letters (uppercase and/or lowercase) plus
  # single spaces between words. Folks like Joseph Gordon-Levitt,
  # Conan O’Brien, and David J. Malan won’t be using your program. (If only!)
  if (nchar(full.name) == 0) {
    stop ("Valid name please")
  }
  isspace  <- integer(0)
  fn.split <- unlist(strsplit(full.name, fixed = TRUE, split = ""))
  isspace  <- which(fn.split == " ")
  init     <- toupper(fn.split[c(1, (isspace+1))])
  paste(init, collapse = "")
}


Environment-specific code in @Async mail method with Spring I have a Spring @Service to asynchronously send emails.  The part that seems to be a bit off to me is some code that is specific to our QA environment.  We want to send and receive all emails through a test email account so that it does not impact live users if we work with real data imported to QA.  I check for the environment and some settings in .properties files and adjust the outgoing email accordingly.

Is there a better way to handle this?

@Async
public void send(String email, String subject, String body) {

    if (email == null) {
        log.error("Not sending; null input: email");
        return;
    }
    try {
        MimeMessage message = this.mailSender.createMimeMessage();
        MimeMessageHelper helper = new MimeMessageHelper(message);
        helper.setFrom(mailSender.getUsername(), getFrom());
        helper.setReplyTo(mailSender.getUsername());
        helper.setSubject(subject);

        //HERE IS THE PART THAT'S IFFY:

        if ("QA".equalsIgnoreCase(EmailManager.getEnvironmentName())) {
            if (isTestMode()) {
                if (!email.equals(getTestEmailAddress())) {
                    helper.setTo(getTestEmailAddress());
                    body += "<br/>This email was originally intended for " + email +
                            " but was redirected to " + getTestEmailAddress() + " for testing.";
                    helper.setText(body, true); //HTML on
                }
            }
        }
        else {
            helper.setTo(email);
            helper.setText(body, true); //HTML on
        }

        if (message == null) {
            log.error("Email message to send is null.  Email={}, Subject={}", email, subject);
            log.error("Body:");
            log.error(body);
            return;
        }
        this.mailSender.send(message);
    }
    catch (MailException | UnsupportedEncodingException | MessagingException e) {
        log.error(e.toString());
        e.printStackTrace();
    }
}

private static String getFrom() {
    Locale defaultLocale = Locale.getDefault();
    ResourceBundle bundle = ResourceBundle.getBundle("messages", defaultLocale);
    return bundle.getString("label.project.name");
}

protected static String getTestEmailAddress() {
    return ResourceBundle.getBundle(AbstractEnvironment.ENVIRONMENT)
            .getString("testEmailAddress");
}

protected static Boolean isTestMode() {
    return Boolean.valueOf(ResourceBundle.getBundle(AbstractEnvironment.ENVIRONMENT)
                                   .getString("redirectAllEmailsToTestEmailAddress"));
}

@Autowired
private JavaMailSenderImpl mailSender;

//setter needed for Junit
public void setMailSender(JavaMailSenderImpl mailSender) {
    this.mailSender = mailSender;
}

Use Spring's dependency injection more aggressively

You are right to be suspicious about hardcoded environment-specific logic.
But you're in luck. This is one of the typical problems that Spring can help you solve.

One way to solve this is to create an interface for controlling the destination email address, and inject an implementation appropriate for the environment. For example:

interface OutgoingEmailController {
    void adjustOutgoingEmail(MimeMessageHelper helper, String email, String body);
}

Implementation to inject in QA:

class QAOutgoingEmailController implements OutgoingEmailController {
    @Override
    public void adjustOutgoingEmail(MimeMessageHelper helper, String email, String body) {
        if (isTestMode()) {
            if (!email.equals(getTestEmailAddress())) {
                helper.setTo(getTestEmailAddress());
                body += "<br/>This email was originally intended for " + email +
                        " but was redirected to " + getTestEmailAddress() + " for testing.";
                helper.setText(body, true); //HTML on
            }
        }
    }
}

Implementation to inject in other environments:

class CommonOutgoingEmailController implements OutgoingEmailController {
    @Override
    public void adjustOutgoingEmail(MimeMessageHelper helper, String email, String body) {
        helper.setTo(email);
        helper.setText(body, true);
    }
}

In your original code you will have an @Autowired OutgoingEmailController outgoingEmailController;, and the IFFY code replaced with outgoingEmailController.adjustOutgoingEmail(helper, email, body);.

Or something like that.

Other remarks

It's odd that you check for null email at the beginning but check for null body at the end. It would be better to move the check on the body further up to the beginning.


Take a number and sum its digits raised to the consecutive powers I am new to Scala have to solve this problem:

  If you have number "xy" should equal  sum of  consecutive powers of its digits

  xy = x^1 + y^2

  For example, take the number 89 and you should have:

  89 = 8^1+ 9^2

  And if we have a list we should filter with the matched numbers based on the above role.

  For example:

  Consider these range of numbers:

  (1, 100) == [1, 2, 3, 4, 5, 6, 7, 8, 9, 89]

  (90, 100) == []

This code works fine, but is this a pure functional programming style or are there some best practices in terms of performance and functionality?

def myFun(str: String):Double ={

   val myDigits=str.toList.map(_.asDigit)

   (myDigits.indices zip myDigits).toMap.map({case(x , y) =>  pow(y,x+1)}).reduce(_+_)

}

(1 to 100 ) .toList.filter({ case(x) => x == myFun(x.toString) })

Recommendations:

For starters I would recommend passing an Int to your function and then converting it to a String before splitting it up and mapping it to its digit components.
Next, Scala has a useful built in function called zipWithIndex that you could use over (myDigits.indices zip myDigits). Granted it will reverse the ordering from what you have, but that can be handled as you can see in the code below.
I would also recommend splitting any map / filter / etc over multiple lines to increase readability.
When you are using reduce to sum the elements of a container you can instead apply the built in sum method.
Finally, I would hold off on converting the (1 to 100) from a Range to a List until after your filter operation is complete.

Code:

def f(i: Int): Int ={
  val myDigits = i.toString.map(_.asDigit)

  myDigits.zipWithIndex.map { case (y, x) =>
    pow(y, x+1).toInt
  }.sum
}

val x = (1 to 100).filter(i => i == f(i)).toList


Is the string a pangram? The following code checks if a string is a pangram or not:

public static class StringExtensions
{
    public static IEnumerable<char> EnglishAlphabet
    {
        get
        {
            for (char i = 'a'; i <= 'z'; i++)
            {
                yield return i;
            }

            yield break;
        }
    }

    /// <summary>
    /// Check if the source string is a pangram.
    /// </summary>
    /// <remarks>
    /// The default dictionary is the english one.
    /// </remarks>
    public static bool IsPangram(this string source)
    {
        return source.IsPangram(EnglishAlphabet);
    }

    /// <summary>
    /// Check if the source string is a pangram.
    /// </summary>
    /// <remarks>
    /// A pangram is a string that uses every letter in the alphabet.
    /// </remarks>
    public static bool IsPangram(this string source, IEnumerable<char> alphabet)
    {
        var lowerAlphabet = new string(alphabet.ToArray()).ToLower();
        IDictionary<char, bool> alphabetCharacters = lowerAlphabet.ToDictionary(c => c, c => false);
        var lowerSource = source.ToLower();

        foreach(char character in lowerSource)
        {
            alphabetCharacters[character] = true;
        }

        return !alphabetCharacters.Values.Contains(false);
    }
}

Do you see anything wrong/weird in it? Any feedback (style, naming, performance) is more than welcome.

Use a set instead of a dictionary

The current algorithm sets the value to true for each character it finds,
and at the end it checks if there is anything still unset. This is a bit tedious, and inefficient. Checking if any value is false is an \$O(n)\$ operation.

A simpler and more natural solution would be using a set data structure (HashSet in C#). You could add characters that you found, and in the end check the size of the set, and compare it with the size of the alphabet.

If the input may contain letters not in the alphabet, then before adding letters to the found set, don't forget to check the letter is actually part of the alphabet. (Thanks @tym32167 for the reminder.)

Use a simple array instead of a dictionary

If the alphabet is always the English alphabet, then an interesting simple alternative is using a simple boolean array of size 'z' - 'a' + 1. As you iterate over the letters, you can derive the array index to use by letter - 'a'. It will be simpler and more storage efficient than using a dictionary.

Generate the alphabet once

The English alphabet is regenerated on each call of IsPangram.
This is unnecessary, as it never changes between calls.
You could generate it once and reuse.


My take at OOP module/prototyping JavaScript The last month I've been reading up on how to take my JavaScript code to the next level. I am a Java programmer at heart, so it feels nice when everything is encapsulated in classes/objects, and these are ready to be instantiated or inhertid or whatever.

I wrote a simple test project with the purpose to refine my techniques, and I'm feeling I can't do anything more with it. Would you guys and gals take a look at it?

var People = People || {};

People = (function() {
    function People() {
        this.people = [];
    }

    People.prototype.addGentleman = function (name) {
        this.people.push(new People.Gentleman(name));
    }

    People.prototype.addLady = function (name, isMarried) {
        this.people.push(new People.Lady(name, isMarried));
    }

    People.prototype.getList = function () {
        var temp = [];

        for (var i = 0; i < this.people.length; i++) {
            temp.push(this.people[i].toString());
        }

        return temp;
    }

    return People;
})();

People.Human = (function () {
    function Human(name, hasLadyParts) {
        this.name = name;
        this.hasLadyParts = hasLadyParts;
    }

    Human.prototype.hasLadyParts = function () {
        return this.hasLadyParts;
    }

    Human.prototype.toString = function () {
        var str = this.name + ' has ';
        if (!this.hasLadyParts) str += 'no ';
        return str + 'lady parts.';
    }

    return Human;
})();

People.Gentleman = (function () {
    function Gentleman (name) {
        People.Human.call(this, name, false);
    }

    Gentleman.prototype = Object.create(People.Human.prototype);

    Gentleman.prototype.toString = function () {
        return 'Mr. ' + People.Human.prototype.toString.call(this);
    }

    return Gentleman;
})();

People.Lady = (function () {
    function Lady(name, hasWeddingRing) {
        People.Human.call(this, name, true);
        this.hasWeddingRing = hasWeddingRing;
    }

    function isNotMarried() {
        return !this.hasWeddingRing;
    }

    Lady.prototype = Object.create(People.Human.prototype);

    Lady.prototype.toString = function () {
        var str = !isNotMarried.call(this) ? 'Mrs. ' : 'Ms. ';
        return str + People.Human.prototype.toString.call(this);
    }

    return Lady;
})();

$(function () {
    var people = new People();
    people.addGentleman('Viktor');
    people.addGentleman('Joakim');
    people.addLady('Mattias', true);
    var list = people.getList();
    var $ul = $('#people');

    for (var i = 0; i < list.length; i++) {
        $ul.append('<li>' + list[i] + '</li>');
    }
});

One question: can I create protected (or public) variables that my child instance can change? Besides that question I only want my code reviewed with regards to my Java context. It feels nice to have it like this, but what can I do better?

The code should be a pretty straight-forward OOP example, and here's a fiddle.


Simple function returning 1 if the Mean = Mode, or 0 if not I'm trying to get better at javascript and have built a simple function that returns 1 if the mode is equal to the mean, and 0 if not. You can enter an array of numbers into the function:

// tests
meanMode([1,2,3]); - returns 0
meanMode([4,4,4,6,2]); - returns 1
meanMode([4,4,4,6,6,6,2]); - returns 0

To clarify, the mean is the middle number in the array, and mode most occurring number.

When I was writing this, I wondered whether I create too many variables and overcomplicate things. The problems I thought I might be making are:

There are too many variables
It's confusing and over complex
It can be hard to retrace my logic and know what the code does and why it's there

I have a few questions that I hope better coders than myself can answer, as I'm relatively new to javascript:

Was using a counts variable necessary?
Are all variable keywords (const & let) optimal?
Is it wise to include comments to describe what the code does?
Was declaring the mode variable before assigning its value the best way? Its value depends on a condition

Before writing this I wrote a brief logic plan as follows:

Mode:

var uniqueVals with unique vals (set)
for each unique value (uniqueVals.map), count no of occurrences in arr (filter.length) & store as an array
if all values are identical, return 0, otherwise get the most frequent & store in variable (there may be cases with no mode)

Mean:

sum all numbers
divide by no of numbers

And:

if unique values length is same as arr length, return 0
if mean = mode, return 1

When people write programming, is it usually best to have a detailed plan to follow? I wonder whether there are any best practices that I could follow to improve my coding ability.

function meanMode(arr) {
    let uniqueVals = Array.from(new Set(arr));
    let counts = uniqueVals.map(function (c, i, a) {
        return arr.filter(function (c2) {
            return c2 == c
        }).length
    }); // [3,1,1]

    if (arr.every(sameValues)) {
        return 0;
    } else {
        var mode;

        // if highest number in counts appears just once, then there is a mode
        if ((counts.filter(function (x) { return x == (Math.max(...counts)) }).length) == 1) {
            mode = uniqueVals[counts.indexOf(Math.max(...counts))]; // scope issue?
        } else {
            return 0;
        }

        const mean = (arr.reduce(function (a, c) { return a + c })) / (arr.length);

        if (mode == mean) {
            return 1;
        } else {
            return 0;
        }
    }

    function sameValues(c, i, a) {
        return c == a[0];
    }
}

First of all, I don’t think this is reasonable:

  if unique values length is same as arr length, return 0

If your array consists of unique values only (e.g. [1, 2, 3, 4]), its mode is not unique, rather there is multiple modes (in this case [1, 2, 3, 4]). How you define what mode exactly means depends on your application so I’m going to ignore this for now.

Let’s look at your code. I’ll list some observations:

Function name. A name like meanEqualsMode would more clearly describe what the function does
Unused variables. There are a few unused variables. Line 3 uses three arguments for the map function’s callback, but the callback is only ever called with one argument: The current array element. You also never use i and a.
Single-letter variable names. This style of naming problematic for a variety of reasons. Most importantly, it’s harder to reason about your own code. For example, in line 21, the reduce function has a and c as callback arguments. sum and count would be better names.
Separation of concerns. Your meanMode function does multiple things. It calculates the mode and the mean. Instead, use separate functions to calculate the mode and mean separately. If your applications uses the check for equality of mode and mean a lot, this would be a third function calling the other two. I’ve done that in the updated code below.
Predictable results. You state that meanMode should return whether the mode and the mean are equal; however, it actually returns a Number (1 or 0). A comparison function (e.g. a function containing the words is, equal, or similar) should always return a boolean value (true or false). This again makes it easier to reason about your code. Just reading meanEqualsMode([2, 3, 5]) should tell you what the result of that function will be without the need to look at the actual implementation.

function meanEqualsMode(array) {
  const mode = arrayMode(array);
  const mean = arrayMean(array);

  return mode === mean;
}

function arrayMode(array) {
  const frequencies = new Map();

  for (const value of array) {
    const currentCount = frequencies.has(value) ? frequencies.get(value) : 0;
    frequencies.set(value, currentCount + 1);
  }

  let highestCount = 0;
  let mostOccurringValue;

  for (const [value, count] of frequencies) {
    if (count >= highestCount) {
      highestCount = count;
      mostOccurringValue = value;
    }
  }

  return mostOccurringValue;
}

function arrayMean(array) {
  return array.reduce((sum, value) => sum + value) / array.length;
}

Now, answering some of your questions:

  Was using a counts variable necessary?

Yes and no. In order to calculate the mode of a list of numbers, you need to know the number of occurrences for each number. However, it is possible to determine the mode while counting the occurences; thus, allowing you to calculate the mode without explicitly storing the occurences. To keep the code simple, I opted for not combining these steps.

  Are all variable keywords (const & let) optimal?

Probably not. You are using const and let. There is very, very little reason to also still use var at all. Most variables are assigned only once and can be defined using const. Some variables have to be declared using let because they’re (re-)assigned later.

  Is it wise to include comments to describe what the code does?

Yes. Take my code for example. Is there something you don’t understand? Then it could’ve maybe explained with a comment. You used a reduce function in your code which I often find hard to read. You need to ask yourself: Is my code dealing with a complex problem that needs explaining or is it just written in a hardly readable fashion? A complex problem is best explained with comments. Code which is hard to read and thus hard to reason about is better fixed by making it more readable and obvious.

  Was declaring the mode variable before assigning its value the best way? Its value depends on a condition

No, it wasn’t. The mode of a list of numbers always exists although it might not be unique. Therefor its value does not logically depend on a condition.


Singly-linked list implementation My code is working now. As you can see in the list.h some type definitions are too long and hard to read. Is there a way to do some short ones? And what happens if the code throws an exception? (I know I will have leaking memory parts) What can I do to collect garbage? And what other functions might be added to code to enhance it?

list.h

#ifndef LIST_H_
#define LIST_H_

#include <iostream>

template <typename T>
class list{
public:
    list();
    ~list();
    class node;
    typedef T           value_type;
    typedef T&          reference;
    typedef node        node_type;
    typedef node_type*  node_pointer;

    list&           push_back(value_type);
    value_type      pop_front();
    node_pointer    begin() const;
private:
    node_pointer    head, tail;
};

template <typename T>
class list<T>::node{
public:
    node(value_type value = 0);
    ~node();
    void            setData(value_type);
    value_type      getData() const;
    void            setNext(node_pointer);
    node_pointer    getNext() const;

private:
    value_type      data;
    node_pointer    ptr_next;
};

template <typename T>
inline list<T>::node::node(value_type value) : data(value), ptr_next(nullptr){
    //std::cout << "*" << value << "\t";
}

template <typename T>
inline list<T>::node::~node(){
    //std::cout << "node dtor" << std::endl;
}

template <typename T>
void list<T>::node::setData(value_type value){
    this->data = value;
}

template <typename T>
typename list<T>::value_type        list<T>::node::getData() const{
    return this->data;
}

template <typename T>
void list<T>::node::setNext(node_pointer ptr){
    this->ptr_next = ptr;
}

template <typename T>
typename list<T>::node_pointer  list<T>::node::getNext() const{
    return this->ptr_next;
}

template <typename T>
list<T>::list() : head(nullptr), tail(nullptr){
    //std::cout << "list ctor" << std::endl;
}

template <typename T>
list<T>::~list(){
    list<T>::node_pointer ptr = this->head, next_ptr;
    while(ptr != nullptr){
        //std::cout << "@" << ptr->getData() << "\t";
        next_ptr = ptr->getNext();
        delete ptr;
        ptr = next_ptr;
    }
}

template <typename T>
list<T>&    list<T>::push_back(value_type value){
    node_pointer item = new node_type(value);
    if(this->tail) this->tail->setNext(item);
    this->tail = item;
    if(!this->head) this->head = this->tail;

    return *this;
}

template <typename T>
typename list<T>::value_type    list<T>::pop_front(){
    if(this->head){
        node_pointer    item = this->head;
        value_type      value = this->head->getData();

        this->head = this->head->getNext();
        delete item;

        return value;
    }

return 0;
}

template <typename T>
typename list<T>::node_pointer list<T>::begin() const{
    return this->head;
}

template <typename T>
std::ostream & operator<<(std::ostream &stream, list<T> &obj){
    typename list<T>::node_pointer ptr = obj.begin();
    while(ptr != nullptr){
        stream << ptr->getData() << ' ';
        ptr = ptr->getNext();
    }
    return  stream;
}

#endif /* LIST_H_ */

main.cpp

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

    list<int> numbers;
    numbers.push_back(3).push_back(5).push_back(11);
    cout << numbers;

    return 0;
}

Let's answer your questions first:

What about the memory issues?
If you are not limited to an older C++ standard in any way, it would be the best to use C++11 (or above), since it provides you with a good implementation of smart pointers (reference, in-depth-tutorial). This way, all your memory management becomes easier and more resilient. Also you do not need to manually delete anything and do not run into trouble if someone else is still accessing the element you just removed.

Caution: Smart pointers and raw pointers do not mix very well, so avoid raw pointers whenever possible.

What about type definitions?
In general, many people nowadays do frown upon typedefs. Firstly, they hide the actual composition of the type and may lead to confusion or wrong assumptions - especially in team projects. Second, in C++ you do have the using keyword which is a better choice here, especially since it can be combined with templates. (reference)

What functions should be provided?
If there is a push_back I would expect some kind of pop_back (also a push_front). Usually you have some methods to give you the number of elements in the list, check if the list is empty... You can take Qt's QList class as an inspiration ().

Suggestion: make node a templates class
Defining the inner class list<T>::node is valid code and gets the job done. However, you may find that it would be more convenient to pull it out of the list class and define a stand-alone class node<T>. A list<T> then contains a node<T>. Depending on your design, you then might find that your node class then is re-usable e.g. for queues.

Recommendation: Use namespaces
Namespaces are your #1 tool to structure your code. At least one level of namespaces should be used at all times to avoid cluttering the global namespace.

Suggestion: use #pragma once
If your compiler supports it, this is a good alternative to the include guards. Just put it into the first line of your header and you are good to go.

Hint: inline is not that important
First, do not waste brain capacity on whether to inline something while developing. Early optimization suffocates progress, diverts attention, and thus kills projects. Get it to run, then optimize one step at a time. Second, modern compilers know best what to inline and what not. Let them do it, they are really good at this. While learning, trust your compiler. You can argue with it later :)

Additional note: Combining templates and inline is used to address issues with the One Definition Rule (ODR). If you define a template, but do not fully specialize it, you may (but do not need to) inline. However if you do also want to provide a fully specialized case, you have to inline, otherwise you conflict with the ODR, since the compiler expands the template and thus already provides an implementation for the case you intended to cover "by hand". See also this question for details and examples. (Kudos to @Cody Gray)

Wrapping things up
I just sketched together a piece of code that sums up the made suggestions/ recommendations to give you an orientation (Note: I prefer the Java-style of camel-casing the type names with an initial upper case letter, but this is a question of personal preference).

#pragma once

#include <memory> // for smart pointers

namespace collection {

template<typename ValueType>
class Node<ValueType> {

private:
    ValueType _data;
    // alternatively store a smart pointer to the data
    // if the amount of data is large e.g. an image
    std::shared_ptr<Node<ValueType>> _next;
    // to "delete" the next pointer just reset it to nullptr
    // only hand out the shared pointer, never the thing it points to
    // the receiver has to check if the pointer is null before use
public:
     //... you know what to do
};

template<typename ValueType>
class List<ValueType> {

private:
    std::shared_ptr<Node<ValueType>> _head;
    // you can also keep track of the tail and whatever...
public:
    // ... all the functions
};

} // end namespace collection

Last but not least
Put your code somewhere accessible (e.g. github) which makes it easier to review code, suggest concrete changes, discuss details, and notify you about the bugs and issues that were found. Post a link and I will have a look at it :)


Returning multiplied matrix array Is there a more clever way of writing this piece of code? It does work but I thought there might be a better way of returning a matrix from multiplying the parameter by itself.

public static void Main(string[] args)
{
    int[,] m1 = CreateMatrix(5);
}

static int[,] CreateMatrix(int rowscols)
{
    int[,] result = new int[rowscols, rowscols];

    int res = rowscols * rowscols;
    int counter= 0;

    for (int i = 0; i < rowscols; ++i)
    {
        for (int k = 0; k < rowscols; ++k)
        {
            result[i, k] = (res - counter);
            counter += 1;
        }
    }

    return result;
}

rowscols is not a very good name.
Although on one hand it's "accurate",
on another it's kind of awkward.
How about dimension or dim instead?

res is also not a very good name.
Since its value is dim * dim,
it's the number of cells in the matrix, so cellCount would be better.

It would be better to move the counter variable inside the loop,
by declaring it in the outer loop together with i,
and incrementing it in the innermost loop together with k.

It's a common practice to name loop variables i, j, k.
You skipped j and went directly for k.
It's not a "problem",
but j would be more natural,
especially because at school (I think) they teach matrices typically with \$m_{ij}\$ notation rather than \$m_{ik}\$

Lastly,
since the result of the function is a matrix,
it will be more meaningful to call the variable matrix instead of result.

Suggested implementation

Putting the above suggestions together:

static int[,] CreateMatrix(int dimension)
{
    var matrix = new int[dimension, dimension];

    var cellCount = dimension * dimension;

    for (int i = 0, counter = 0; i < dimension; ++i)
    {
        for (int j = 0; j < dimension; ++j, ++counter)
        {
            matrix[i, j] = cellCount - counter;
        }
    }

    return matrix;
}

Special thanks to @Abbas for pointing out to use the var keyword instead of explicitly declaring the matrix and cellCount variables.


Create hash generator with configurable hash algorithm Create hash generator with configurable hash algorithm. Also need method to create salt. Implemented in visual studio 2015. Must work in DNX Core 5.0.

 public class HashGenerator : IHashGenerator
    {
        // Algorithm has to inherit from HashAlgorithm base class.
        // For example, another algorithm that can be used is: "System.Security.Cryptography.MD5";
        private const string HashAlgorithmName = "System.Security.Cryptography.SHA256";

        public string GenerateHash(string input, string salt)
        {
            var saltedPassword = Encoding.UTF8.GetBytes($"{input}{salt}");

            var hashAlgorithm = GetHashAlgorithm();
            var hashedPassword = hashAlgorithm.ComputeHash(saltedPassword);

            return Convert.ToBase64String(hashedPassword);
        }

        public string CreateSalt()
        {
            var buffer = new byte[25];
            using (var rng = RandomNumberGenerator.Create())
            {
                rng.GetBytes(buffer);
            }
            return Convert.ToBase64String(buffer);
        }

        private HashAlgorithm GetHashAlgorithm()
        {
            var typeInfo = Type.GetType(HashAlgorithmName).GetTypeInfo();
            var createMethod = typeInfo.GetDeclaredMethods("Create").First(f => f.GetParameters().Count() == 0);
            var hashAlgorithm = (HashAlgorithm)createMethod.Invoke(null, null);
            return hashAlgorithm;
        }
    }

Right now you're stuck with the use of a constant. GetHashAlgorithm should receive, as a parameter, a Type. You then have 3 options for method signature :

The first one has the advantage of being sure that the Type inherits HashAlgorithm.

string GetHashAlgorithm<T>() where T : HashAlgorithm
string GetHashAlgorithm(Type hashAlgorithmType)
string GetHashAlgorithm(string typeName)

You shouldn't use a constant, as it is... constant. You then need to recompile your code to change the algorithm, which isn't good.

You could retrieve this string from your app.config file, which means you could configure it without recompiling.

If you don't mind being stuck with constant algorithm, I still don't think you should const them. Retrieve the HashAlgorithm from a factory maybe :

public class HashAlgorithmFactory
{
    public HashAlgorithm Create(/*Parameters to decide which algorithm maybe?*/)
    {
        //Conditions to decide which algorithm to return according to parameter
        return SHA256.Create();
    }
}

Also, is there a reason why you don't use your method CreateSalt in your CreateHash method instead of providing a salt parameter? I think it would make sense, but I don't have full knowledge of your needs.


Memory management with password retrieval Questions:

Is this code secure? (I think that it is, but I'm a newbie so I want to be sure.)
Is the get_pass function correct with passing the arguments to the free_memory function?
Do I have to delete the pass' buffer with memset?

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <termios.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <netinet/in.h>

void get_pass(char **host, char **user, char **pass);
void free_memory(char *h, char *u, char *p);

int main(){

    char *host, *user, *pass;

    host = (char *) calloc(64, sizeof(char)); /* spazio per max 64 caratteri e inizializzo a 0 (maggior sicurezza) */
    if(!host){
        fprintf(stdout, "\nErrore di allocazione della memoria\n");
        exit(EXIT_FAILURE);
    };

    user = (char *) calloc(64, sizeof(char));
    if(!user){
        fprintf(stdout, "\nErrore di allocazione della memoria\n");
        exit(EXIT_FAILURE);
    };

    pass = (char *) calloc(64, sizeof(char));
    if(!pass){
        fprintf(stdout, "\nErrore di allocazione della memoria\n");
        exit(EXIT_FAILURE);
    };

    /* Immissione di hostname, username e password.
     * Controllo inoltre i 'return code' dei vari fscanf e, se non sono 0, esco.
     * Per evitare buffer overflow imposto limite massimo a 64 caratteri
     */
    fprintf(stdout,"--> Inserisci hostname (max 64 caratteri): ");
    if(fscanf(stdin, "%63s", host) == EOF){
        fprintf(stdout, "\nErrore, impossibile leggere i dati\n");
        free_memory(host,user,pass);
        exit(EXIT_FAILURE);
    }
    fprintf(stdout,"\n--> Inserisci username (max 64 caratteri): ");
    if(fscanf(stdin, "%63s", user) == EOF){
        fprintf(stdout, "\nErrore, impossibile leggere i dati\n");
        free_memory(host,user,pass);
        exit(EXIT_FAILURE);
    };
    fprintf(stdout, "\n--> Inserisci password (max 64 caratteri): ");
    get_pass(&host,&user,&pass);

    /* Stampo a video le informazioni immesse */
    fprintf(stdout, "\n\nHost: %s\nUser: %s\nPass: %s\n\n", host,user,pass);

    free_memory(host,user,pass);

    return EXIT_SUCCESS;
}

void get_pass(char **host, char **user, char **pass){
    /* Grazie a termios.h posso disabilitare l'echoing del terminale (password nascosta) */
    struct termios term, term_orig;
    tcgetattr(STDIN_FILENO, &term);
        term_orig = term;
        term.c_lflag &= ~ECHO;
        tcsetattr(STDIN_FILENO, TCSANOW, &term);
        /* Leggo la password e controllo il 'return code' di fscanf */
        if(fscanf(stdin, "%63s", *pass) == EOF){
        fprintf(stdout, "\nErrore, impossibile leggere i dati\n");
        tcsetattr(STDIN_FILENO, TCSANOW, &term_orig);
        free_memory(*host, *user, *pass);
        exit(EXIT_FAILURE);
    };
        /* Reimposto il terminale allo stato originale */
        tcsetattr(STDIN_FILENO, TCSANOW, &term_orig);
}

void free_memory(char *h, char *u, char *p){
    /* Libero la memoria occupata */
    free(h);
    free(u);
    free(p);
}

I'm not sure it's exactly insecure, but I'd consider it excessively verbose and repetitive. Given that your user, host and pass are all small, fixed-size, and allocated for the duration of a single function, dynamic allocation seems to gain little (and cost quite a bit of verbosity) with them. I think I'd write something like this:

void getprompt(char *string, size_t max, char const *prompt) {
    char buffer[16];

    sprintf(buffer, "%%%ds", max-1);
    printf("%s", prompt);
    if (scanf(buffer, string) == EOF) {
        printf("\nErrore, impossibile leggere i dati\n");
        exit(EXIT_FAILURE);
    }
}

int main(){
    char host[64], user[64], pass[64];

    getprompt(host, sizeof(host), "--> Inserisci hostname (max 64 caratteri): ");
    getprompt(user, sizeof(user), "\n--> Inserisci username (max 64 caratteri): ");

    printf("\n--> Inserisci password (max 64 caratteri): ");
    get_pass(&host,&user,&pass);

    /* Stampo a video le informazioni immesse */
    printf("\n\nHost: %s\nUser: %s\nPass: %s\n\n", host,user,pass);

    return EXIT_SUCCESS;
}

There also seems to be no reason to pass host or user to get_pass -- it doesn't really use them (and logically, shouldn't). I'd probably write it as a wrapper around getprompt -- do the tcsetattr, call getprompt, then undo the tcsetattr. Although you have matched the sizes correctly in this case, your get_pass has implicit knowledge of the size of pass; it works as-is, but is relatively fragile, so in the long term it would be fairly easy for somebody to create a hole by adjusting the size in get_pass but not in main (or vice versa).

I'd also consider changing getprompt to return a bool indicating success/failure rather than having it directly exit the program. Along with the suggested change to getpass, that would turn your main into something like:

int main() {
    char host[64], user[64], pass[64];

    if (getprompt(host /* ... */) &&
        getprompt(user /* ... */) &&
        get_pass(pass /* ... */)
    {
        printf("\n\nHost: %s\nUser: %s\nPass: %s\n\n", host, user, pass);
    }
    return EXIT_SUCCESS;
};

Bottom line: I don't see any obvious security holes in the code as it stands, but it strikes me as excessively verbose and rather fragile. It's unnecessarily difficult to be certain that it's correct now, and likely to get broken in the long term even if it is all correct now.


BIOS Video Service INT 10H (functions 9 and 13H) functionality On the way to protected or long modes, real mode code needs to provide a lot of visual feedback. Video services INT 10H provide this functionality, but lack fluidity. This algorithm provides write character and attribute AH = 9 and write string AH = 13H functionality, reading NULL terminated strings.

ENTER

;    BH = Video page
;    BL = Attribute
;    CX = Repeat count or NULL to read string to EOS
;    DH = Row    (0 - 24)
;    DL = Column (0 - 79)
; DS:SI = Pointer to ASCIIZ string or character to be repeated CX times

LEAVE

;    CX = Number of characters displayed
; ES:SI = Points to the next position after null. Might be next string

To achieve a result like this:

The string would be defined as:

  Title:    db  'Proto_Sys 1.00.0', 0, 0xc4

and called by:

mov     si, Title
mov     dx, 0x11e       Line 2, Column 32 (values zero indexed)
mov     bx, 13          Light Magenta
xor     cx, cx
call    528H
mov     cl, 18          Repeat 18 times
mov     bl, 15          Bright White
add     dx, 255         Row 3, Column 30
call    528H

NOTE: As this is in a boot loader, there is a lot of code before this that initializes pertinent registers.

Begin with setting ES:DI to point to proper place in video memory B8000 - BFFFF:

528  31C0      xor  ax, ax
52A  88FC      mov  ah, bh
52C  80C4B8    add  ah, VIDEO_SEG
52F  06        push     es
530  8EC0      mov  es, ax
532  88D4      mov  ah, dl
534  C1E807    shr  ax, 7
537  57        push     di
538  89C7      mov  di, ax
53A  B0A0      mov  al, 0xa0
53C  52        push     dx
53D  C1EA08    shr  dx, 8
540  F7EA      imul     dx
542  5A        pop  dx
543  01C7      add  di, ax

Read the first character and then determine if this character is to be repeated CX times or the first of a NULL-terminated string:

547  AC        lodsb
548  09C9      or   cx, cx
54A  7407      jz   553H

Do repeat process and return to caller:

54C  51        push     cx
54D  F3AB      rep  stosw
54F  59        pop  cx

; see comments restored out of order
; -------------------------------
; 550  07        pop    es   [di] 5F
; 551  5F        pop    di   [es] 07
; -------------------------------

552  C3        ret

Keep reading until null and update CL as it returns the length of the string:

553  08C0      or     al, al
555  74F9      jz     550H
557  FEC1      inc    cl
559  AB        stosw
55A  AC        lodsb
55B  EBF6      jmp     553H

Some corrections

The LEAVE comments mention ES:SI where you intended DS:SI
If the comment for mov dx, 0x11e talks about "Line 2", then it should also say "Column 31" (in stead of "32")

add     dx, 255         Row 3, Column 30

This is a very nice approach! Although for reviewing purposes I had to read it a few times over, it is definitely better here than the simpler mov dx, 0x021d would have been.

Optimizations

When registers need to be preserved, it will be better to group the pushes and pops together. You've put them amongst the rest of the instructions and maybe because of this introduced the error of inverse restoration. A trick I've been using for many years now is numbering my push and pop instructions (see below). This facilitates enormously discovering these errors.

push    es            ;(1)
push    di            ;(2)

The calculation of the ES segment register can be shorter and clearer.

mov     ax, VIDEO_SEG ;0xB800
add     ah, bh        ;BH=Display page
mov     es, ax

The calculation for the video address in DI can be optimized a lot. Next code does it in 14 bytes, where your code used 18 bytes. Also it uses only half the instructions.

movzx   ax, dh        ;DH=Row
imul    ax, 80        ;80 characters per row
add     al, dl        ;DL=Column
adc     ah, 0         ;(*)
imul    di, ax, 2     ;Double for character+attribute AND bring in DI

(*) If you know that the counter in CX will always be reasonably small, then maybe write adc ah,ch here and shave off another byte.
In order to decide what the function should do you test the CX register using or cx, cx jz 553H. This can easily be shortened to jcxz 553H. A 2 byte gain.
Incrementing the resulting counter in CX will be shorter using inc cx because that's a 1-byte instruction.
The loop currently uses 2 jump instructions per iteration. This is wasteful in terms of speed and can easily be avoided as you'll see in below summary.

In summary

  push    es            ;(1)
  push    di            ;(2)
  mov     ax, VIDEO_SEG ;0xB800
  add     ah, bh        ;BH=Display page
  mov     es, ax
  movzx   ax, dh        ;DH=Row
  imul    ax, 80        ;80 characters per row
  add     al, dl        ;DL=Column
  adc     ah, 0         ;(*) Possibly use "adc ah, ch" with CH=0
  imul    di, ax, 2     ;Double for character+attribute AND bring in DI
  lodsb
  jcxz    Test          ;Till end of string marker
Char:
  push    cx            ;(3)
  rep     stosw
  pop     cx            ;(3)
  jmp     Exit
String:
  inc     cx
  stosw
  lodsb
Test:
  test    al, al
  jnz     String
Exit:
  pop     di            ;(2)
  pop     es            ;(1)
  ret


Count the letter repetition in a string Given a String. we have to print out the letter repetition

Sample input "aaaabbaac"

Sample output "4a2b2a1c"

public void continousWords(String input) {
String sample= input +" ";
String[] sampleList  =   sample.split("");
String temp = sampleList[0];
String result="";
int placeHolder=0;
for(int i=0;i<sampleList.length;i++){

    if(temp.equalsIgnoreCase(sampleList[i])) {
        placeHolder++;

    }
    else {
      result = result+placeHolder+temp;
      placeHolder =1;
      temp = sampleList[i];
    }

}
System.out.println(result);

}

Is there a better way to approach the problem

This method doesn't need any member of it's class, thus it can be declaced static.

You want to count the repetation of each letter, not the frequency in the string, don't you? At least, that's what I understood from your example result 4a2b2a1c where a is included twice. If you want the frequency 6a2b1c the algorithm would be different.

AN alternative approach to your proposal would be to convert the String in a char array and to analyze that.

public static void continousWords(String input) {
    char[] chars = input.toLowerCase().toCharArray();
    StringBuilder b = new StringBuilder();

    for (int i = 0, count; i < chars.length; i += count) {
        count = 1;
        while (i + count < chars.length && chars[i] == chars[i + count])
            count++;
        b.append(count).append(chars[i]);
    }

    String result = b.toString();
    System.out.println(result);
}


"Suspicious comparison of integer references" while checking for a monotonically increasing sequence I have the following code which iterates on an Integer[] and makes sure that the the values in the array are in ascending order and that there are no neighbouring duplicates:

Integer previous = null;
for (Integer tb : trancheBoundaries) {
    if (previous == tb) {
        throw new IllegalThresholdConfigurationException("Two boundaries are the same");
    } else if (previous != null && previous > tb) {
        throw new IllegalThresholdConfigurationException("Previous is larger than current - tranche boundaries must be in ascending order");
    } else {
        previous = tb;
    }
}

I am getting a warning from Sonar warning of "suspicious comparison of integer references" on the first if clause.

I know why I am getting this, but in a situation like this, is it bad practice? I think it's ok because I know that I will always be comparing the same instances.

You should only use == if you work with elementary data types (boolean, byte, char, short, int, long, float, long).
If you use == on instances of classes like Integer you check if it is the same instance of the class not if the value is the same. You hardly ever want to check if two references refer to the same instance. That is especially true for the primitive wrappers.
So as a rule of thump:

use == for boolean, byte, char, short, int, long, float, long
use equals(..) otherwise

For little values of wrapper types == might work, but this totally depends on the VM implementation. JLS 5.1.7 Boxing Conversion:

Ideally, boxing a given primitive value p, would always yield an identical reference. In practice, this may not be feasible using existing implementation techniques. The rules above are a pragmatic compromise. The final clause above requires that certain common values always be boxed into indistinguishable objects. The implementation may cache these, lazily or eagerly. For other values, this formulation disallows any assumptions about the identity of the boxed values on the programmer's part. This would allow (but not require) sharing of some or all of these references.
This ensures that in most common cases, the behavior will be the desired one, without imposing an undue performance penalty, especially on small devices. Less memory-limited implementations might, for example, cache all char and short values, as well as int and long values in the range of -32K to +32K.


Linked list implemented as classes, not imperative code As an update to previous code I've submitted:

Improvements this time are mainly having everything moved to a class, before I plough on and add more functionality (I've come back to C++ after a few years away and this is a revision exercise).

#include <iostream>
#include "LinkedList.h"

class LinkedList
{
public:
    LinkedList(void);
    ~LinkedList(void);
    void traverseList();
    int length();
    void push(int data);

private:
    struct node{
    int data;
    node *pointee;
    };

    node *head;
    node *last;
    int count;
};

LinkedList::LinkedList(void)
{
        head = new node();
    last = new node();
    head->data = 0;
    head->pointee = last;
    last->data = 0;
    last->data = NULL;
    count = 2;
}

LinkedList::~LinkedList(void)
{
}

/*
    To be replaced with an iterator and an overloaded print operator for output
*/
void LinkedList::traverseList(){
    for(node *iterator = LinkedList::head ; iterator ; iterator = iterator->pointee)
    {
        std::cout << iterator->data << std::endl;
    }
}

int LinkedList::length(){
    return LinkedList::count;
}

void LinkedList::push(int data){

    node *newNode = new node();
    newNode->data = data;
    newNode->pointee = NULL;
    last = newNode;
    ++count;
}

int main(){

    LinkedList list;

    for(int i=1 ; i<4 ; i++)
    {
        list.push(i);
    }

    int length = list.length();
    std::cout << length << std::endl;
    list.traverseList();
    return 0;
}

Since we have no clean implementations:

The thing to notice is that the push_back() and erase() become trivial if you use a sentinal. This is because you do not need to test for NULL in any part of the code.

    Node(Node* after)
    {
        prev          = after;
        next          = after->next;
        prev->next    = this;
        next->prev    = this;
    }
    ~Node()
    {
        prev->next  = next;
        next->prev  = prev;
    }

    iterator push_back(T const& value)
    {
        tail    = new ValueNode(value, tail);

        // If new succeeded then increment count
        ++count;
        return iterator(tail);
    }

    void erase(iterator const& i)
    {
        // Get the value we will set as tail if the delete works
        Node*   newTail = (tail == i.node)
                            ? i.node->prev
                            : tail;
        delete i.node;
        // Delete worked now we can update the state.
        tail    = newTail;
        --count;
    }

Here is the whole code

#ifndef THORS_ANVIL_UTILITY_LINKED_LIST
#define THORS_ANVIL_UTILITY_LINKED_LIST

#include <algorithm>
#include <functional>

template<typename T>
class LinkedList
{
    struct Node
    {
        Node*   prev;
        Node*   next;
        Node(Node* after)
        {
            prev          = after;
            next          = after->next;
            prev->next    = this;
            next->prev    = this;
        }
        ~Node()
        {
            prev->next  = next;
            next->prev  = prev;
        }
        void swap(Node& rhs) throw()  { std::swap(prev, rhs.prev); std::swap(next, rhs.next);}
    };
    struct ValueNode: Node
    {
        ValueNode(T const& value, Node* after)
            : Node(after)
            , data(value)
        {}
        T       data;
    };

    Node    head;
    Node*   tail;
    int     count;

    public:
        struct Iterator: std::iterator<std::input_iterator_tag, T, ptrdiff_t, const T*, const T&>
        {
            Iterator(Node* node): node(node){}

            T&       operator*()        {return  static_cast<ValueNode*>(node)->data;}
            T*       operator->()       {return &static_cast<ValueNode*>(node)->data;}
            T const& operator*()  const {return  static_cast<ValueNode*>(node)->data;}
            T const* operator->() const {return &static_cast<ValueNode*>(node)->data;}

            Iterator& operator++()     /*prefix*/  { node = node->next; return *this;}
            Iterator& operator--()     /*prefix*/  { node = node->prev; return *this;}
            Iterator  operator++(int)  /*postfix*/ { Iterator result(*this);this->operator++(); return result;}
            Iterator  operator--(int)  /*postfix*/ { Iterator result(*this);this->operator--(); return result;}

            bool operator !=(Iterator const& rhs) const    { return node != rhs.node;}
            bool operator ==(Iterator const& rhs) const    { return !(*this != node);}

            mutable Node*   node;
        };

        LinkedList()
            : tail(&head)
            , count(0)
        {}

        ~LinkedList()
        {
            clear();
        }

        void clear()
        {
            while(head.next != &head)
            {
                Node*   old = head.next;
                head.next   = head.next->next;
                --count;
                delete old;
            }
        }

        LinkedList(LinkedList const& copy)
            : tail(&head)
            , count(0)
        {
            std::for_each(copy.head->next, copy.head, std::bind1st(std::mem_fun(&LinkedList::push_back), this));
        }
        LinkedList& operator=(LinkedList rhs)
        {
            rhs.swap(*this);
            return *this;
        }
        void swap(LinkedList& rhs) throw()
        {
            std::swap(head,     rhs.head);
            std::swap(tail,     rhs.tail);
            std::swap(count,    rhs.count);
        }

        bool    isEmpty()   const { return &head == tail; }
        size_t  size()      const { return count; }

        typedef Iterator           iterator;
        typedef Iterator const     const_iterator;

        iterator push_back(T const& value)
        {
            tail    = new ValueNode(value, tail);
            ++count;
            return iterator(tail);
        }

        void erase(iterator const& i)
        {
            Node*   newTail = (tail == i.node)
                                ? i.node->prev
                                : tail;
            tail    = newTail;
            --count;
            delete i.node;
        }

        iterator begin()                {return iterator(head.next);}
        const_iterator begin() const    {return iterator(head.next);}
        iterator end()                  {return iterator(&head);}
        const_iterator end() const      {return iterator(&head);}
};
#endif

Then Main

#include <LinkedList.h>
#include <iostream>

int main()
{
    LinkedList<int> list;
    list.push_back(5);
    LinkedList<int>::iterator item = list.push_back(7);
    for(LinkedList<int>::iterator loop = list.begin(); loop != list.end(); ++loop)
    {
        std::cout << *loop << "\n";
    }
    list.erase(item);
    for(LinkedList<int>::iterator loop = list.begin(); loop != list.end(); ++loop)
    {
        std::cout << *loop << "\n";
    }
}

Diagram of Node Insertion:

Node Before Insertion Starts

      after
      #########                         #########
   -->#  Next #------------------------>#  Next #----
      #       #                         #       #
   ---#Prev   #<------------------------#Prev   #<---
      #########                         #########

                       NewNode(this)
                       #########
                       #   Next#
                       #Prev   #
                       #########

prev          = after;

      after
      #########                         #########
   -->#  Next #------------------------>#  Next #----
      #       #                         #       #
   ---#Prev   #<------------------------#Prev   #<---
      #########   |                     #########
                  |
                  |    (this)
                  |    #########
                  |    #   Next#
                  -----#Prev   #
                       #########

next          = after->next;

      after
      #########                         #########
   -->#  Next #------------------------>#  Next #----
      #       #                  |      #       #
   ---#Prev   #<-----------------|------#Prev   #<---
      #########   |              |       #########
                  |              |
                  |    (this)    |
                  |    ######### |
                  |    #   Next#--
                  -----#Prev   #
                       #########

prev->next    = this;

      after
      #########                         #########
   -->#  Next #------            ------>#  Next #----
      #       #     |            |      #       #
   ---#Prev   #<----|------------|------#Prev   #<---
      #########   | |            |      #########
                  | |            |
                  | |  (this)    |
                  | |  ######### |
                  | -->#   Next#--
                  -----#Prev   #
                       #########

next->prev    = this;

      after
      #########                         #########
   -->#  Next #------            ------>#  Next #----
      #       #     |            |      #       #
   ---#Prev   #<--- |            |   ---#Prev   #<---
      #########   | |            |   |  #########
                  | |            |   |
                  | |  (this)    |   |
                  | |  ######### |   |
                  | -->#   Next#--   |
                  -----#Prev   #<-----
                       #########


Adding multiple items of different categories to one object I want to run the function addItem() for every item. Multiple items will belong to the same slot (category). For each category I have to run the function with the same slot. I don't want to manually write down the slot for each and every item, because there could be 100's for each category.

The goal is to get the best result in terms of: readability, maintainability and effort to add items.

Here is my current code:

let equipment = {
    head: {},
    cape: {},
    neck: {},
    ammunition: {},
    weapon: {},
    body: {},
    shield: {},
    legs: {},
    hand: {},
    feet: {},
    ring: {}
};

function createShortName(name) {
    return name.toLowerCase().replace(/\s/g, '');
}

function addItem(slot, name, cost, img, income=0, atk_bonus=0, str_bonus=0, def_bonus=0, rngd_bonus=0, mage_bonus=0) {

    let newItem = {
        slot: slot,
        name: name,
        cost: cost,
        img: img,
        income: income,
        atk_bonus: atk_bonus,
        str_bonus: str_bonus,
        def_bonus: def_bonus,
        rngd_bonus: rngd_bonus,
        mage_bonus: mage_bonus
    }

    equipment[slot][createShortName(name)] = newItem;
}

let currentSlot = 'head';
addItem(currentSlot, 'head_item_1', 100, 'img');
addItem(currentSlot, 'head_item_2', 200, 'img');

currentSlot = 'cape';
addItem(currentSlot, 'cape_item_1', 100, 'img');

currentSlot = 'neck';
addItem(currentSlot, 'neck_item_1', 100, 'img');
addItem(currentSlot, 'neck_item_2', 200, 'img');
addItem(currentSlot, 'neck_item_3', 400, 'img');
addItem(currentSlot, 'neck_item_4', 800, 'img');

// etc. etc. etc.

As you can see, I add items to the equipment object at the bottom of the code. Is there a better approach?

With regards to the last couple of lines, one thing you could do from a functional perspective is to bind the first argument like so:

function bindFirst(func, firstArgument) {
    return function(...arguments) {
        return func(firstArgument, ...arguments);
    }
}

const addToHead = bindFirst(addItem, 'head');

addToHead('head_item_1', 100, 'img');
...

You could also use data structure like a Map and loop through it:

let newItems = new Map([
    [
        'head',
        [{
            name: 'head_item_1',
            cost: 100,
            img: 'img'
        },
        {
            name: 'head_item_1',
            cost: 100,
            img: 'img'
        }]
    ], [
        'cape',
        [{
            name: 'cape_item_1',
            cost: 100,
            img: 'img'
        },
        {
            name: 'cape_item_1',
            cost: 200,
            img: 'img'
        }]
    ]
]);

items.forEach((items, slot) => {
    const addToSlot = bindFirst(addItem, slot);
    items.forEach(item => {
        addToSlot(item.name, item.cost, item.img);
    });
});

Have you considered that maybe equipment and item should be classes? A class called Equipment could have a method like addItem(slot, item) where item is an object of type Item, which has a constructor with a similar signature to your addItem function.