Using twitter4j to fetch usernames that retweeted me I'm using using twitter4j to fetch usernames that retweeted me. I'm interested in any feedback that make the code more elegant, and also that might reduce the number of calls I'm making to the Twitter API.

import java.util.ArrayList;
import twitone.structure.BaseTwitterClass;
import twitone.structure.TwitApplicationFactory;
import twitter4j.Paging;
import twitter4j.Status;
import twitter4j.Twitter;
import twitter4j.TwitterException;

public class getmyretweeters extends BaseTwitterClass {

    public static void main(String[] args) throws TwitterException {

        Twitter twitter = TwitApplicationFactory.getjoereddingtonTwitter();
        String temp[] = getRetweeters(twitter);
        for (String string : temp) {
            System.out.println(string);
        }
    }

    public static String[] getRetweeters(Twitter twitter) {
        ArrayList<String> names = new ArrayList<String>();
        try {
            for (Status status : twitter.getUserTimeline(new Paging(1, 200))) {
                System.out.println(status.getText());
                if (status.getRetweetCount() > 0) {
                    Thread.sleep(12000);// Because I don't want to breach
                                        // Twitter's rate limits
                    for (Status rt : twitter.getRetweets(status.getId())) {
                        names.add(rt.getUser().getScreenName());
                    }
                }
            }
        } catch (TwitterException e) {
            e.printStackTrace();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        return names.toArray(new String[names.size()]);
    }

}

General Notes

Follow Java naming conventions.  Class names should have EachWordCapitalized, and variable and method names should be inCamelCase.  Also, class names should be nouns, and method names should be verbs.
For collections, the overarching interface is usually used.  (i.e., List<String> names = new ArrayList<String>();)  Not really that important in my view, but it adds some style points according to some authorities.
Most of the int literals should be true constants.  So for example, you would have private static final long WAIT_TIME = 12000; at the top of your class, and then call Thread.sleep(WAIT_TIME).  It doesn't matter for this small application, but for large-scale projects this will allow you to change the setting globally just by modifying this one value.  Otherwise, you'll be doing a lot of digging through your code, and you're guaranteed to forget to change that one spot it mattered most.  Every.  Single.  Time.
Why are you bothering to convert the ArrayList<String> into a String[] before returning it?  You may as well just change the return type of the method to List<String>.  In general, it's best to avoid primitive arrays and use these data structures.  And they can also be used in the enhanced for loop in your main method.
You should hardly ever swallow Exceptions and do nothing about them.  Again, it's pretty trivial in this case and doesn't really matter, but if you're looking to develop good programming habits, start logging and doing safety checking and all that now.  It'll make your life a lot easier down the road.

Also, this method name made me die a little on the inside: getjoereddingtonTwitter()


Simplify this function which finds the coordinates of the neighbors of an isometric cell I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id

const getNeighborsCellId = cellId => {
    const neighbors = []
    const {x, y} = getCoordinatesFromCellId(cellId)

    // LOL ->

    const isStaggered = y % 2
    if (isStaggered){

      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))

    }
    else {
      neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))
    }

    return neighbors
}

First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?

Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.

The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.

const CELL_NEIGHBORS = [
   // Even:
   [{dx: 0, dy: -1}, {dx: 0, dy: -2}, {dx: +1, dy: -1} /* etc. */ ],
   // Odd:
   [ /* TODO */ ]
];

function getAllNeighborCoordinates({x, y}) {
   const evenOdd = y % 2;

   return CELL_NEIGHBORS[evenOdd].map({dx, dy} => {x: x + dx, y: y + dy});
}

Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:

 function getNeighborsCellId(cellId) {
   return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
     .map(getCellIdFromCoordinates);
 }

(DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)


Shorten This Code with... blocks? I think this code cannot be shortened much, especially because I only consider two cases. But maybe there's a language construct that I don't know that would help.

if (subControl.inverted) {
    newValue = subControl.value+(float)(1.0f/(float)subControl.decorator.ticks);
    if (newValue > 1)
        newValue = 0;
} else {
    newValue = subControl.value-(float)(1.0f/(float)subControl.decorator.ticks);
    if (newValue < 0)
        newValue = 1;
}

Some of the (floats) are gratuitous, which is irrelevant.

Edit: The code could look like this, which helps. However, I guess this is the result of reading the answers already ;)

if (subControl.inverted) {
    newValue = subControl.value+(float)(1.0f/(float)subControl.decorator.ticks);
} else {
    newValue = subControl.value-(float)(1.0f/(float)subControl.decorator.ticks);
}

if (newValue > 1)
    newValue = 0;
if (newValue < 0)
    newValue = 1;

Not a lot shorter, but a bit...

float ticks = (float)(1.0f/(float)subControl.decorator.ticks);
newValue = subControl.value + subControl.inverted ? ticks : - ticks;

if (subControl.inverted && newValue > 1)  {
    newValue = 0;
} else if (!subControl.inverted && newValue < 0){
    newValue = 1;
}

You could make it even shorter if the you are restricting the values to just 0 and 1.

EDIT: you say in a comment, that it should be between 0 and 1, so...

if (newValue > 1)  {
    newValue = 0;
} else if (newValue < 0){
    newValue = 1;
}

Which is fewer characters if not fewer lines, and imo clearer in intent.


Complex class polymorphism and lazy-loaded components I'm having an issue defining a base class and child classes, which handle initialization differently. I want it to work in ES6 but allow babel to transpile to ES5, and currently babel does not have a backwards compatible translation for new.target so I'm looking for something "close enough".

class Complex {
  constructor(real, imag, abs, arg) {
    if (new.target === Complex) {
      real = arguments.length > 0 && real !== undefined ? real : 0;
      imag = arguments.length > 1 && imag !== undefined ? imag : 0;
      abs  = arguments.length > 2 && abs  !== undefined ? abs  : Math.sqrt(real * real + imag * imag);
      arg  = arguments.length > 3 && arg  !== undefined ? arg  : Math.atan2(imag, real);

      this.real = real;
      this.imag = imag;
      this.abs  = abs >= 0 ? abs : -abs;
      // limits arg between (-pi, pi] and also corrects phase for negative abs
      this.arg  = arg ? -((-arg + (abs >= 0 ? Math.PI : 0)) % (Math.PI * 2) - Math.PI) : abs < 0 ? Math.PI : 0;
    }
  }

  // ... all mathematical static and non-static methods will go here
}

Complex.Cartesian = class Cartesian extends Complex {
  constructor(real = 0, imag = 0, abs, arg) {
    super();

    this.real = real;
    this.imag = imag;

    if (arguments.length === 4) {
      Object.defineProperties(this, {
        abs: {
          configurable: true,
          enumerable: true,
          value: abs >= 0 ? abs : -abs,
          writable: true,
        },
        arg: {
          configurable: true,
          enumerable: true,
          value: arg ? -((-arg + (abs >= 0 ? Math.PI : 0)) % (Math.PI * 2) - Math.PI) : abs < 0 ? Math.PI : 0,
          writable: true,
        },
      });
    }
  }

  get abs() {
    const value = Math.sqrt(this.real * this.real + this.imag * this.imag);

    Object.defineProperty(this, 'abs', {value, configurable: true, enumerable: true, writable: true});

    return value;
  }

  get arg() {
    const arg = Math.atan2(this.imag, this.real);
    const value = arg ? -((-arg + Math.PI) % (Math.PI * 2) - Math.PI) : 0;

    Object.defineProperty(this, 'arg', {value, configurable: true, enumerable: true, writable: true});

    return value;
  }
};

Complex.Polar = class Polar extends Complex {
  constructor(abs = 0, arg = 0, real, imag) {
    super();

    this.abs = abs >= 0 ? abs : -abs;
    this.arg = arg ? -((-arg + (abs >= 0 ? Math.PI : 0)) % (Math.PI * 2) - Math.PI) : abs < 0 ? Math.PI : 0;

    if (arguments.length === 4) {
      Object.defineProperties(this, {
        real: {
          configurable: true,
          enumerable: true,
          value: real,
          writable: true,
        },
        imag: {
          configurable: true,
          enumerable: true,
          value: imag,
          writable: true,
        },
      });
    }
  }

  get real() {
    const value = this.abs * Math.cos(this.arg);

    Object.defineProperty(this, 'real', {value, configurable: true, enumerable: true, writable: true});

    return value;
  }

  get imag() {
    const value = this.abs * Math.sin(this.arg);

    Object.defineProperty(this, 'imag', {value, configurable: true, enumerable: true, writable: true});

    return value;
  }
};

The general idea is to have a base class Complex() that handles initialization of all complex components (real, imag, abs, arg) when invoked directly, but not when invoked by child classes Cartesian() or Polar(), which use lazy-loaded abs and arg, or real and imag respectively.

My idea is to use this instanceof Object && this.constructor === Complex instead of new.target, but I'm not sure if that handles all edge cases exactly the same.

Any other advice not directly related to my specific issue is also welcome.

The code above is under MIT license, so if you plan on "borrowing" it for anything, please provide credit accordingly.

I ended up side-stepping the issue entirely by extending the Complex class with ES5 prototype chaining instead of ES6 extends:

class Complex {
  constructor(real = 0, imag = 0, abs = Math.sqrt(real * real + imag * imag), arg = Math.atan2(imag, real)) {
    this.real = real;
    this.imag = imag;
    this.abs  = abs >= 0 ? abs : -abs;
    this.arg  = arg ? -((-arg + (abs >= 0 ? Math.PI : 0)) % (Math.PI * 2) - Math.PI) : abs < 0 ? Math.PI : 0;
  }

  // ...
};

const Cartesian = Complex.Cartesian = function Cartesian(real = 0, imag = 0) {
  this.real = real;
  this.imag = imag;
};

Cartesian.prototype = Object.create(Complex.prototype, {
  constructor: {
    configurable: true,
    enumerable: false,
    value: Cartesian,
    writable: true,
  },
  abs: {
    configurable: true,
    enumerable: true,
    get() {
      const value = Math.sqrt(this.real * this.real + this.imag * this.imag);

      Object.defineProperty(this, 'abs', {
        configurable: true,
        enumerable: true,
        value,
        writable: true
      });

      return value;
    },
    set(value) {
      Object.defineProperty(this, 'abs', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
  },
  arg: {
    configurable: true,
    enumerable: true,
    get() {
      const arg = Math.atan2(this.imag, this.real);
      const value = arg ? -((-arg + Math.PI) % (Math.PI * 2) - Math.PI) : 0;

      Object.defineProperty(this, 'arg', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
    set(value) {
      Object.defineProperty(this, 'arg', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
  },
});

const Polar = Complex.Polar = function Polar(abs = 0, arg = 0) {
  this.abs = abs >= 0 ? abs : -abs;
  this.arg = arg ? -((-arg + (abs >= 0 ? Math.PI : 0)) % (Math.PI * 2) - Math.PI) : abs < 0 ? Math.PI : 0;
};

Polar.prototype = Object.create(Complex.prototype, {
  constructor: {
    configurable: true,
    enumerable: false,
    value: Polar,
    writable: true,
  },
  real: {
    configurable: true,
    enumerable: true,
    get() {
      const value = this.abs * Math.cos(this.arg);

      Object.defineProperty(this, 'real', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
    set(value) {
      Object.defineProperty(this, 'real', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
  },
  imag: {
    configurable: true,
    enumerable: true,
    get() {
      const value = this.abs * Math.sin(this.arg);

      Object.defineProperty(this, 'imag', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
    set(value) {
      Object.defineProperty(this, 'imag', {
        configurable: true,
        enumerable: true,
        value,
        writable: true,
      });

      return value;
    },
  },
});

This allowed me to write the constructors without requiring the call to super(), which was what created the issue in the first place.


Counting the number of primes in C and C++11 I found this old piece of code that I wrote while I was learning C a while back:

#include <stdio.h>
#include <stdlib.h>
#include <math.h>

int main(int argc, char *argv[]) {
    if (argc < 2) {
        fprintf(stderr, "Usage: %s <N>\n", argv[0]);
        return 1;
    }

    size_t limit = strtoul(argv[1], (char **) NULL, 10);
    size_t i, j, res = 0;
    size_t isqrt = (size_t) sqrt(limit);
    char* numbers = calloc(limit, 1);

    for (i = 2; i <= isqrt; i++) {
        if (numbers[i]) continue;
        for (j = i*i; j < limit; j += i) numbers[j] = 1;
    }

    for (i = 2; i < limit; i++) {
        if (!numbers[i]) ++res;
    }
    printf("%lu\n", res);
    free(numbers);
    return 0;
}

It's a sieve of Erathosthenes. Since now I started learning C++, I decided to rewrite it in C++11:

#include <iostream>
#include <memory>
#include <vector>
#include <string>
#include <algorithm>
#include <math.h>

int main(int argc, char *argv[]) {
    if (argc < 2) {
        std::cerr << "Usage: " << argv[0] << " <N>" << std::endl;
        return 1;
    }

    size_t limit = std::stoul(argv[1]);
    size_t i, j, res = 0;
    size_t isqrt = size_t (sqrt(limit));
    std::unique_ptr<std::vector<bool> > sieve(new std::vector<bool>(limit));

    for (i = 2; i <= isqrt; ++i) {
        if (sieve->at(i)) continue;
        for (j = i*i; j < limit; j += i) sieve->at(j) = 1;
    }

    for_each(sieve->cbegin() + 2, sieve->cend(), [&res](bool i) { res += !i; });
    std::cout << res << std::endl;
    return 0;
}

Is this the most idiomatic way to code in C++? I also tried to replace the first for loop with a for_each and a functor that accepts sieve, but in the end I decided it wasn't worth it and left it like that.

In both versions I am allocating on the heap because the sieve array/vector gets too big for the stack.

Interestingly, the C++ version is faster! I thought they were pretty much identical, but instead the C++ version wins by a large margin:

$ gcc --std=c99 -O3 erat.c -lm -o erat
$ time ./erat 1000000000
50847534
./erat 1000000000  11.50s user 0.41s system 99% cpu 11.927 total
$ g++ --std=c++14 -O2 erat.cpp -o erat2
$ time ./erat2 1000000000
50847534
./erat2 1000000000  8.52s user 0.02s system 99% cpu 8.554 total

Feedback on the C version is also welcomed!

Idiomatic C++ Review
#include <math.h>

C++ has its own version of math header.
#include <cmath>

Difference is all the functions are placed in the std namespace.
Prefer not to use std::endl.
        std::cerr << "Usage: " << argv[0] << " <N>" << std::endl;

Forcing a flush is not normally what you want to do. The streams will automatically flush when required and doing it manually will only make the code less efficient.
Prefer to put one variable per line. Also for loop counters can be declared inside the for statement (see below).
    size_t i, j, res = 0;

Don't dynamical allocate when a local variable will do:
    std::unique_ptr<std::vector<bool> > sieve(new std::vector<bool>(limit));

    // Should be
    std::vector<bool> sieve(limit);

Also std::vector<bool> is special. It is optimized for space not speed. It also has some other quirks that make it undesirable, so prefer std::vector<char> unless you really really need to save space. BUT because of the space saving you can get better caching which may improve time. In this case presumably because of the size of the vector keeping it as bool makes it much more efficient.
see: Comparing std::vector to std::vector
Declare loop variables inside the for:
    for (i = 2; i <= isqrt; ++i) {

    // Usually written like this:
    for(int i = 2; i <= isqrt; ++i) {

The at() method is a checked access to the member. It validates your index is in range (you don't do this in the C code).
        if (sieve->at(i)) continue;

Unless you are using unvalidated user input there is little need to use the at() method, prefer to use operator[].
In C++14 the functions std::begin() and std::end() were introduced to get the iterators for containers. You should prefer to use these as they work with arrays as well as the standard containers.
    for_each(sieve->cbegin() + 2, sieve->cend(), [&res](bool i) { res += !i; });

    // Can be written as:
    auto begin = std::begin(sieve);
    std::advance(begin, 2);
    for_each(begin, std::end(sieve), [&res](bool i) { res += !i; });

Don't bother with a return at the end of main.
    return 0;


Function to map letters to numbers I have a method that converts characters to numbers.
It expects a single alphabetical character and returns the equivalent number.
For example, if A is provided it returns 1, for B it returns 2.
If an array is passed, it returns the message "array not accepted".

function characterToNumberCoverter(character) {
  if (Array.isArray(character)) {
    return { status: "failure", message: "array not accepted" };}
  character = character.toUpperCase();
  let myRegExp = /[A-Z]/;
  if (myRegExp.test(character)) {
    return character.charCodeAt(0) - 64;}}

The method returns an object if the input is invalid and an integer if the input is valid. The method is invoked like this:

let intConvertedInt=characterToNumberCoverter('A');

This works fine when the input is valid. What is a better way to handle the use case where an invalid input is passed? Example in which the result will be an object:

    let intConvertedInt=characterToNumberCoverter({});

Are there any other function design patterns that I should be following?

Why not just compare directly to the charcode?

function characterToNumberCoverter(character) {
  let code = character.toUpperCase().charCodeAt(0);
  if (code > 64 && code < 91) return code - 64
  return 0
}


Recursive spinlock for C using ASM, revision #1 Precursor for this revision here in code-review:

Recursive spinlock for C using ASM

Improvements have been made thanks to forsvarir.

Now I am trying to optimize this lock so it becomes more practical.

The code in question.

#define gate_Gate volatile int
#define gate_Pass volatile int

extern inline void
gate_Enter
(gate_Gate *gate, gate_Pass *pass)
{
    asm volatile (
        "jmp gate_Enter_check\n"
        "gate_Enter_wait:\n"
        "pause\n"
        "gate_Enter_check:\n"
        "cmp %[lock], %[pass]\n"     // Skip if pass >= lock
        "jge gate_Enter_skip\n"
        "mov %[lock], %%eax\n"       // Spinlock start
        "cmp %[gate], %%eax\n"       // Check if locked without lock cmd
        "je gate_Enter_wait\n"  // If locked, wait.
        "lock xchg %%eax, %[gate]\n" // Lock seems to be free, now we attempt to take it.
        "test %%eax, %%eax\n"
        "jnz gate_Enter_wait\n" // Spinlock end
        "gate_Enter_skip:\n"
        "add %[lock], %[pass]\n"     // Checkin pass
        : [gate] "=m" (*gate), [pass] "=m" (*pass)
        : [lock] "r" (1)
        : "eax"
    );
}

extern inline int
gate_Leave
(gate_Gate *gate, gate_Pass *pass)
{
    if (*pass <= 0) {
        // Signal underflow for debugging.
        return -1;
    }
    asm volatile (
        // Cleaner solution thanks to forsvarir/stackoverflow
        "add %[checkout], %[pass]\n"
        "jnz gate_Leave_skip\n"
        "mov %[unlock], %[gate]\n"
        "gate_Leave_skip:\n"
        : [gate] "=m" (*gate), [pass] "=m" (*pass)
        : [unlock] "r" (0), [checkout] "r" (-1)
        : "eax"
    );
    return 0;
}

// Example use. Like forsvarir pointed out, we have to be careful and keep pass on the stack.
void exampleUse(int count, gate_Pass pass)
{
    int underflow;
    printf("pass=%d, gate=%d\n", pass, gate);
    gate_Enter(&gate, &pass);
    if (count == 3) {
        // pass = 0; // force deadlock.
    }
    if (count < 5) {
        printf("Count = %d\n", count);
        exampleUse(++count, pass);
    }
    underflow = gate_Leave(&gate, &pass);
    if (!underflow) {
        printf("pass=%d, gate=%d\n", pass, gate);
    } else {
        printf("underflow detected\n");
    }
}
gate_Pass pass = 0;
exampleUse(0, pass);

The question

The intent of this code is a lock that allows recursion and does so efficiently.

Is there a point for this optimization? Am I right to assume that "lock" operator is very expensive and this approach is justified and right?
Is my attempt to early? Locking is hard to test. For this reason I strongly believe that spotting an error is a valid candidate for an answer.
Finally if all is well, there is always something to improve. This is a code review! Feel free to review the code as you see fit.

gate_Leave has a superfluous compiler hint:

    : "eax"

Since gate_Leave returns an int and eax is often (always?) used to return values this is probably costing you extra instructions (although it might not as the function is inline, depending on how clever your compiler is).

Indentation and Labels

Labels are quite important in assembly.  I'd consider adding indentation so that they are easier to find, a lot like you would indent the contents of a while loop.

asm volatile (
    "jmp gate_Enter_check\n"
"gate_Enter_wait:\n"
    "pause\n"
"gate_Enter_check:\n"
    "cmp %[lock], %[pass]\n"     // Skip if pass >= lock
    "jge gate_Enter_skip\n"
    "mov %[lock], %%eax\n"       // Spinlock start
    "cmp %[gate], %%eax\n"       // Check if locked without lock cmd
    "je gate_Enter_wait\n"  // If locked, wait.
    "lock xchg %%eax, %[gate]\n" // Lock seems to be free, now we attempt to take it.
    "test %%eax, %%eax\n"
    "jnz gate_Enter_wait\n" // Spinlock end
"gate_Enter_skip:\n"
    "add %[lock], %[pass]\n"     // Checkin pass
    : [gate] "=m" (*gate), [pass] "=m" (*pass)
    : [lock] "r" (1)
    : "eax"
);

I'd consider minimising the contents of your spin section.  Some of the operations only need to be performed once (loading eax, checking pass).  I'm not sure you're really getting much benefit from the unlocked gate check, but without running tests I don't know either way:

asm volatile (
    "cmp %[lock], %[pass]\n" // We only need to check this once, if
                             // this changes while we are in the lock,
                             // some other thread is messing with it and
                             // we are in trouble anyway.
    "jge gate_Held\n"
    "mov %[lock], %%eax\n"   // Only need to load eax once, if it is ever
                             // changed, it's to 0 because we've just
                             // locked the gate

"gate_Enter_check:\n"
    "lock xchg %%eax, %[gate]\n" // Attempt to take it.
    "test %%eax, %%eax\n"
    "jz gate_Held\n"
    "pause\n"
    "jmp gate_Enter_check\n"

"gate_Held:\n"
    "add %[lock], %[pass]\n"
    : [gate] "=m" (*gate), [pass] "=m" (*pass)
    : [lock] "r" (1)
    : "eax"
);


Reduce list manipulation for QGIS layer tree I am developing a plugin for the GIS software, QGIS. The code below reads the number of layers in various groups (as shown in the image) and adds them to a QTableWidget:

For each group, I want to count the number of layers, divide 1 by this number and then add the result of this to each of the layers in each group in the table.

Taking "Group1" as an example:

Count the number of layers (in this case 3).
Calculate 1 / 3 = 0.33....
Insert 0.33... to the first three layers in the table.
Repeat for the remaining groups.

So the table looks like this:

However, there's a fair bit of list manipulation involved which seems unnecessary so was hoping to see if there's a way to reduce this.

Here is the code:

# Define QTableWidget
qTable = self.dockwidget.tableWidget
# Define group
root = QgsProject.instance().layerTreeRoot()
main_group = root.findGroup('Main group')

def refresh_table():
    # Define parameters for QTableWidget
    qTable.setRowCount(0)
    # Define list to contain all layer names
    layer_data = []
    # Define list to contain the number of layers
    layer_count = []

    # Find all groups in main_group
    for group in main_group.children():
        # Define list to count number of layers in each group
        layer_list = []
        # Find all layers in each group
        for child in group.children():
            node = root.findLayer(child.layer().id())
            try:
                # If layer is visible, add them to lists
                if node.isVisible() == Qt.Checked:
                    layer_data.append(child.layerName())
                    layer_list.append(child.layerName())
            except AttributeError:
                pass
        # Insert the number of layers in each group to layer_count list
        layer_count.append(len(layer_list))

    # Get total number of layers
    layer_data_count = len(layer_data)
    try:
        # Create new list for layer_count but ignore any zeros
        new_layer_count = [x for x in layer_count if x != 0]
    except ValueError:
        pass

    # List manipulation
    # Calculate the number of layers in each group and divide 1 by this number
    value_list = [1 / float(x) for x in new_layer_count]
    # Format the list to one decimal place
    formatted_value_list = ['%.1f' % elem for elem in value_list]
    # Convert the values in list to float
    formatted_value_list_to_float = [float(x) for x in formatted_value_list]
    # Create final list containing each layer and the values of their group
    final_value_list = [x for n,x in zip(new_layer_count,formatted_value_list_to_float) for _ in range(n)]

    # Set number of rows/columns
    nb_row = len(layer_data)
    nb_col = 2
    qTable.setRowCount(nb_row)
    qTable.setColumnCount(nb_col)
    # Hide row index number
    qTable.verticalHeader().setVisible(False)

    # Insert layer names and values
    for row in range(layer_data_count):
        for col in [0]:
            item = QTableWidgetItem(str(layer_data[row]))
            qTable.setItem(row,col,item)
            # Make first column non-editable
            item.setFlags(QtCore.Qt.ItemIsEnabled)
        for col in [1]:
            item = QTableWidgetItem(str(final_value_list[row]))
            qTable.setItem(row,col,item)

Your lists manipulations are indeed messy. It seems like you are trying to perform multiple things at once and thus you end up mixing variables that serve different purposes into the same loops.

Instead, you should try to extract logical steps to perform, such as:

extract informations you need from the internals of QGis to a similar but simpler representation;
compute the data you need out of this simplified representation;
format the data you computed for presentation;
use the formatted data to update your visual presentation.

Here the second and third step are simple enough they can be made at once.

But first, we will need an helper function to simplify the inner working of the first double for loop:

def is_visible_layer(layer, root):
    node = root.findLayer(layer.id())
    try:
        return node.isVisible() == Qt.Checked
    except AttributeError:
        return False

Now I’m not QGis expert, but I’m wondering if the root is really necessary to get the meta-information about the node. Isn't it possible to write it like:

def is_visible_layer(layer):
    try:
        return layer.isVisible() == Qt.Checked
    except AttributeError:
        return False

?

This let us write the for loop like:

for group in main_group.children():
    layer_list = []
    for child in group.children():
        if is_visible_layer(child.layer(), root):
            layer_data.append(child.layerName())
            layer_list.append(child.layerName())
    layer_count.append(len(layer_list))

Which can be simplified using list-comprehensions:

for group in main_group.children():
    layer_list = [child.layerName() for child in group.children() if is_visible_layer(child.layer(), root)]
    layer_data.extend(layer_list)
    layer_count.append(len(layer_list))

Now recall what I said about performing several things at once? Here you already try to count things while extracting the informations from the QGis internals. Let's get that for later and simplify the loop once again to build a list of list of layer names:

layers = [
    [
        child.layerName()
        for child in group.children()
        if is_visible_layer(child.layer(), root)
    ]
    for group in root.findGroup(main_group).children()
]

and then, only after you can start counting. And as it is much easier using this structure, you can start converting the list of list into a single list of couples that will mimic the table layout:

table = [
    (layer, len(group))
    for group in layers
    for layer in group
]

Turning this list of couples into an N × 2 table is then really easy. The whole code should look like:

def is_visible_layer(layer, root):
    node = root.findLayer(layer.id())
    try:
        return node.isVisible() == Qt.Checked
    except AttributeError:
        return False

def refresh_table(root, qTable, main_group='Main group'):
    # Get names of relevant layers, as a list of list of names
    layers = [
        [
            child.layerName()
            for child in group.children()
            if is_visible_layer(child.layer(), root)
        ]
        for group in root.findGroup(main_group).children()
    ]

    # Convert the layer list to the table layout i.e. a list of couples
    table = [
        (layer, len(group))
        for group in layers
        for layer in group
    ]

    # Configure output table
    qTable.setRowCount(len(table))
    qTable.setColumnCount(2)
    qTable.verticalHeader().setVisible(False)

    # Insert data into table
    for row, (layer, count) in enumerate(table):
        name_item = QTableWidgetItem(layer)
        name_item.setFlags(QtCore.Qt.ItemIsEnabled)
        count_item = QTableWidgetItem('{:.1f}'.format(1./count))
        qTable.setItem(row, 0, name_item)
        qTable.setItem(row, 1, count_item)

Call it like

refresh_table(
    QgsProject.instance().layerTreeRoot(),
    self.dockwidget.tableWidget)


File / Folder Tally Tool I'm trying to learn C++ having come from a C/Assembly background.  I thought one of the first tools I'd make as a project is a "directory tally" program:

It will count all the files in the current directory
If the user specifies, it will count all the folders in the current directory too
If the user specifies, it will count all the files in subdirectories as well (and folders if the user specifies)

Given that most of my academic, professional, and personal programming projects have involved C instead of C++ I'm pretty sure that my code is not idiomatic C++ (yet), but I have used guidelines like use std::(w)string over (w)char(_t) * strings, use the STL as much as possible, etc.

I'm wondering if I can get feedback on:

Is it obvious from looking at my code that I come from a C background?
What can I do to make this program look more like idiomatic C++?
The usual code review stuff, things I could write more clearly or bad things I'm doing that I should avoid.

#include <iostream>
#include <filesystem>

namespace fs = std::experimental::filesystem;

bool __cdecl recur_flag(std::wstring arg)
{
    if (arg.length() < 2) return false;
    return ((arg.at(0) == L'-' || arg.at(0) == L'/') && (arg.at(1) == L'r' || arg.at(1) == L'R'));
}

bool __cdecl dir_flag(std::wstring arg)
{
    if (arg.length() < 2) return false;
    return ((arg.at(0) == L'-' || arg.at(0) == L'/') && (arg.at(1) == L'd' || arg.at(1) == L'D'));
}

bool __cdecl help_flag(std::wstring arg)
{
    if (arg.length() < 2) return false;
    return ((arg.at(0) == L'-' || arg.at(0) == L'/') && (arg.at(1) == L'?'));
}

int __cdecl wmain(int argc, wchar_t *argv[])
{
    bool recur = false, dircount = false;
    unsigned long long count = 0;

    recur = ((argc >= 2) && (recur_flag(std::wstring(argv[1])))) || ((argc >= 3) && (recur_flag(std::wstring(argv[2]))));

    dircount = ((argc >= 2) && (dir_flag(std::wstring(argv[1])))) || ((argc >= 3) && (dir_flag(std::wstring(argv[2]))));

    if ((argc >= 2) && (help_flag(std::wstring(argv[1]))) || (argc >= 3) && (help_flag(std::wstring(argv[2]))))
    {
        std::wcout << L"Usage: dircount /R /D /?" << std::endl << L"\t/R Recursive - count files in current directory and subdirectories" << std::endl <<
            L"\t/D Include directories when counting files" << std::endl << L"\t/? Display this help message" << std::endl << std::endl;
        return 0;
    }

    if (recur)
    {
        for (auto &p : fs::recursive_directory_iterator(fs::current_path()))
        {
            if (fs::is_directory(p))
            {
                if (dircount) count++;
            }
            else count++;
        }
    }
    else
    {
            for (auto &p : fs::directory_iterator(fs::current_path()))
            {
                if (fs::is_directory(p))
                {
                    if (dircount) count++;
                }
                else count++;
            }
    }

    std::wcout << L"There are " << count << L" items in the current directory"; if (recur) std::wcout << L" tree"; std::wcout << std::endl;

    return 0;
}

I'm sure someone else can provide a more comprehensive answer than I could, Just a few things:

string::at throws an out-of-range exception, since you're not actually catching anywhere, I'd use [] since at() is slower and you're already checking the string length, not that it really matters that much for this simple code though.
I would factor out the std::wstring(argv[1]) and similar into local variables so there are fewer brackets.
if statements without braces are less maintainable.
wmain is windows specific, so this code won't compile elsewhere.


Convert bitcoin value based on exchange rate from JSON API I’m learning F# and trying to find a more ‘functional’ way to code a simple program that retrieves the price of BTC and calculates the EUR value of an amount of Bitcoin.

open System
open System.Net
open Newtonsoft.Json.Linq

let myBTC = 0.1234567
let client = new WebClient()
client.UseDefaultCredentials = true
let priceInfo = client.DownloadString("https://blockchain.info/ticker")
let jPrice = JObject.Parse priceInfo
let eurPrice = float (jPrice.["EUR"].["buy"] :?> JValue)
let calcWorth = myBTC * eurPrice
printfn "%s" calcWorth.ToString("N")

My take is that there is no harm in adding a few named functions. Also it is probably better to ignore the result of setting a standard .net property.

open System
open System.Net
open Newtonsoft.Json.Linq

let downloadPriceInfo () =
    let client = new WebClient()
    client.DownloadString("https://blockchain.info/ticker")

let getPriceOfEuro (price: JObject) = float (price.["EUR"].["buy"] :?> JValue)

let euroToBtc euro = 0.1234567 * euro

let btcOfEuro =
    downloadPriceInfo ()
    |> JObject.Parse
    |> getPriceOfEuro
    |> euroToBtc

printfn "%s" (btcOfEuro.ToString("N"))


Moving repeated files from one folder to another I want to scan my whatsapp images folder and move all the repeated images to folder named recycle bin to delete them later:

<?php

$dir    = 'C:\wamp\www\whatsapp';
$files = scandir($dir);

$x = 0;

foreach($files as $f1)
{

    $crc1 = strtoupper(dechex(crc32(file_get_contents("whatsapp/".$f1))));

    unset($files[$x]);

    $j = 0;

    foreach($files as $f2)
    {

        $crc2 = strtoupper(dechex(crc32(file_get_contents("whatsapp/".$f2))));

        if($crc1 == $crc2){

            rename("whatsapp/".$f2, "recycle bin/".$f2);

            unset($files[$j]);
        }

        $j++;
    }

    $x++;
}

exit('Done');

Does this code seems to be trusted to move only the repeated images without any mistakes?

Maybe you try with this pseudocode:

get files list from directory
declare array for crc
for each file in directory
check if the mime content type is correct
get crc for file
check if this crc is in array for crc
if is in array move it
else push crc into array

Thanks Pimgd. You are right. I should write it on the beginning. First of all there is only one loop, so if you have a lot of files, it will be faster. Second, it checks content file, so you will not touch not image files. Third, the code will be more clear.


S3 Bash Tools Part 3 This is a follow on to a previous question:
Part 3: User Scripts
All the code I am submitting here is available in github

The reason to write these tools is so I can prototype and quickly verify that I am doing things correctly before implementing them in C++. This is mainly necessary because the documentation these scripts are based on are relatively bad and confusing.

The scripts used by users:
s3cp
#!/usr/bin/env bash

badflag=0
declare -a ARGS

function usage {
    echo "Usage:"
    echo "s3cp  <file> <s3-url> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]"
    echo "s3cp  <s3-url> <file> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]"
    echo
    echo "s3-url:  s3://<bucket>/<Object-Path>"
    exit 1
}

verbose=''
dryrun=0
access=${AMZ_ACCESS_KEY}
secret=${AMZ_SECRET_KEY}
for var in "$@"; do
    if [[ "${var}" == '--verbose' ]]; then
        verbose='/dev/stdout'
        continue
    fi
    if [[ "${var}" == '--dryrun' ]]; then
        dryrun=1
        continue;
    fi
    if [[ "${var%=*}" == "--key" ]]; then
        access=${var#*=}
        continue;
    fi
    if [[ "${var%=*}" == "--secret" ]]; then
        secret=${var#*=}
        continue;
    fi
    flag=${var#-}
    if [[ "${flag}" != ${var} ]]; then
        badflag=1
        echo "Bad flag: ${var}"
        continue
    fi
    ARGS+=("$var")
done

if [[ "${access}" == "" || "${secret}" == "" ]]; then
    echo "Error: No Access or Secret provided."
    echo "These can be set in the environment variables AMZ_ACCESS_KEY/AMZ_SECRET_KEY or passed with flags"
    usage
fi

if [[ ${#ARGS[@]} != 2 || ${badflag} != 0 ]]; then
    usage
fi

src=${ARGS[0]}
dst=${ARGS[1]}

doUpload=0
doDownload=0
srcS3=${src#s3://}
if [[ "${src}" == "${srcS3}" ]]; then
    doUpload=1
fi
dstS3=${dst#s3://}
if [[ "${dst}" == "${dstS3}" ]]; then
    doDownload=1
fi

if [[ ${doUpload} == 0 && ${doDownload} == 0 ]]; then
    echo "Neither source or destination is on S3. Just use normal cp"
    usage
fi
if [[ ${doUpload} == 1 && ${doDownload} == 1 ]]; then
    echo "Both source or destination is on S3. Don't support move yet."
    usage
fi

dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
source ${dir}/s3/signature

if [[ ${doUpload} == 1 ]]; then
    source ${dir}/s3/upload
    upload "${src}" "${dst}" "${access}" "${secret}" '' "${verbose}" "${dryrun}"
else
    source ${dir}/s3/download
    download "${src}" "${dst}" "${access}" "${secret}" '' "${verbose}" "${dryrun}"
fi

In case of errors/warnings, write your output to stderr and not just echo them to stdout stream. This might also include you returning the usage section.

Having a chain of if block is imo, always very bad. You can use a switch-case block there, and keep shifting the arguments. A simple example would be:

ARGS=()
while [[ $# -gt 0 ]]
do
    key="$1"

    case $key in
      -v|--verbose)
        verbose='/dev/stdout'
        shift # past argument
        ;;
      --dryrun)
        dryrun=1
        shift # past argument
        ;;
      -k=*|--key=*)
        access="${key#*=}"
        shift # past argument
        shift # past value
        ;;
      -k|--key)
        shift # past argument
        access="$1"
        shift # past value
        ;;
      .
      .
      .
      *)    # unknown option
        if [[ "${key#-}" != ${key} ]]; then
            badflag=1
            >&2 echo "Bad flag: ${var}"
        fi
        ARGS+=("$1") # save it in an array for later
        shift # past argument
        ;;
    esac
done

Notice how easily you can maintain the --key=KEY and --key KEY format parameters. Apart from that, it is really easy to later on add another new argument to your utility.

Keep defining your globals in UPPER_CASE, so that it easy to distinguish them later in your ever-expanding code.

echo "These can be set in the environment variables AMZ_ACCESS_KEY/AMZ_SECRET_KEY or passed with flags"

AWS recommends having the environment variables named AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY. I would consider it breaking the norm if you are not respecting defaults.

Define the usage content (or man page?) as a heredoc:

USAGE=<<EOF

Usage:
s3cp  <file> <s3-url> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]
s3cp  <s3-url> <file> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]

s3-url:  s3://<bucket>/<Object-Path>

EOF


Access an associative array value given an array of keys in PHP I have an object which stores an app configuration in an associative array, as this:

[
  'Database' =>
    [
      'DB_SERVER' => 'localhost'
      'DB_NAME' => 'adbname'
      'DB_USER' => 'theuser'
      'DB_PASS' => 'thepass'
    ]
]

It could have more than two levels.

And a function, get(), which receives a variable number of arguments and returns the configuration value for this parameters. For example, Config::get('Database','DB_PASS') will return 'thepass'.

This is an excerpt of the code:

class Config
{
    protected static $values;

    ...

    public static function get()
    {
        $val = &self::$values;
        $argList = func_get_args();
        for ($i = 0; $i < count($argList); $i++) {
            $val = &$val[$argList[$i]];
            if(empty($val)) break;
        }
        return (is_null($val)?"":$val);
    }
}

Is there a more elegant / efficient way of accessing the value?

(note: I posted the question here but I was notified this web was a proper place to ask)

Your code seems fine, but it's a bit verbose. You can make it more elegant with a foreach loop, clean up the way you exit the function, and if you're using PHP 5.6 or above you can use a real variable-length argument list. See:

http://php.net/manual/en/functions.arguments.php#functions.variable-arg-list

class ConfigurationHandle
{
    protected static $configuration;

    <...>

    public static function get(...$arguments)
    {
        $value = &self::$configuration;
        foreach($arguments as $argument)
        {
          $value = &$value[$argument];
          if(empty($value)) return "";
        }
        return $value;
    }
}

I've written out full words, because there is no need to abbreviate. I think this code is more elegant, because it is easier to read and understand, but it is not more efficient.

It would be more efficient if you dumped the Config class altogether, and used a one dimensional array like this:

$configuration = ['DATABASE_SERVER' => 'localhost',
                  'DATABASE_NAME'   => 'adbname',
                  'DATABASE_USER'   => 'theuser',
                  'DATABASE_PASS'   => 'thepass'];

That is immeasurable more efficient, and might I say it's elegant too?

If top speed is not important to you, but multiple levels, and more control, are, then go for the class.


Feedback on Erlang Lists processing function I have just started learning erlang...I am just practicing the recursion and all in erlang...I tried to write a few of the list processing functions on my own in erlang...Can someone review the code and give some advice on it...

-module(listma).
-export([duplicates/1,reverse/1])
%% Remove the duplicates in a list

duplicates(List) ->
    duplicates(List,[]).

duplicates([Head|Tail],Newlist) ->
    Insert =
        fun(H,N) ->
            case [H] -- N of
                [] -> N;
                [H] -> [H|N]
            end
        end,
    duplicates(Tail,Insert(Head,Newlist));

duplicates([],Outputlist) ->
    reverse(Outputlist).

%% Reverse a list

reverse(List) ->
    reverse(List,[]).
reverse([Head|Tail],Newlist) ->
    reverse(Tail,[Head|Newlist]);
reverse([],Outputlist) ->
    Outputlist.

Your Insert lambda doesn't use any context so you could rewrite it as a top-level function or move its body to the clause.
And you could use lists:member/2 instead of --.

I'd write first clause like this:

duplicates([Head | Tail], Acc) ->
   NewAcc = case lists:member(Head, Acc) of
               true ->
                  Acc;
               _ ->
                  [Head | Acc]
            end,
   duplicates(Tail, NewAcc);


A method to insert a database entry for a newly registered account I've started to learn an OOP and I've built a class called accountactions, and I would like to know if I did a good job writing it.

The class is in the file: accountactions.class.php.

<?php

class accountactions(){

    public function register($login, $password, $email){

        //Zapisujemy dane wysłane formularzem
        $this->username = mysql_real_escape_string($login);
        $this->password = mysql_real_escape_string($password);
        $this->email = mysql_real_escape_string($email);

        //Hash password
        $this->password = md5(sha1($this->password));

        $db->simplequery("INSERT INTO radio_users(id, username, password, email) VALUES('', '$this->username', '$this->password', '$this->email')");

    }

}

?>

The register.php file:

<?php

    require_once("accountactions.class.php");

    $account = new accountactions();

    $account->register('samplelogin', 'samplepassword', 'sample@email');

?>

And I am having some issues with this fragment:

$db->simplequery("INSERT INTO radio_users(id, username, password, email) VALUES('', '$this->username', '$this->password', '$this->email')");

How do I join my db class to my account class?

I'd like to keep a model where I can do something like:

$account->register('$_POST['login']', '$_POST['password']', '$_POST['email']');

Unless there is a better way to do this.

Im newbie in OOP so any tips and guidelines are appreciated.

Organization, while not exactly code related, is still very important. Especially when it comes to OOP and especially when you're just getting started. If your code is organized correctly, there is no need for namespaces such as ".class". I spent a solid week perusing different frameworks before finally deciding on one I liked. Then I spent another month fine tuning and tweaking it. Originally, my organizational scheme came from the Zend framework.

application
    configs
    controllers
    models
    views
data
    logs
    cache
public
    images
    css
    js

There's a bit more to it, but I got the most important bits. This scheme specifically caters to an MVC framework, so it may not apply to your situation. You don't have to use this scheme, you don't even have to use a popular one. Hell, you can create your own to fit your own needs, and sometimes that is required. However, a good organizational scheme is hard to find and the popular ones are usually popular for a reason. And one thing a good organizational scheme should do is allow you to avoid namespacing your files. In my scheme my class files would go in one of two folders. The "controllers" or the "models". Knowing these were the only locations for class files means I don't have to namespace them.

Sorry for the mini-rant, let's move along now. I don't really see anything wrong with the OOP, but then again, there's not really much going on here. I do have a few suggestions that might help though.

First, your class is incomplete. You have three class properties and have not declared them. You should always declare your properties, otherwise you risk allowing public access to private properties, such as the password property.

class accountactions() {
    private
        $username,
        $password,
        $emal
    ;

You should avoid internal comments as they just add clutter to your code. If your code is self-documenting then most comments become unnecessary anyways. Everything else can be relegated to doccomments. So, for instance: "Hash password" is pretty obvious from the context and is unnecessary.

Its a minor violation, but still, there is a violation of the "Don't Repeat Yourself" (DRY) Principle here. As the name implies, your code should not repeat itself. You use the same function on each of your method's parameters in order to sanitize them. The easiest way to do this would be to change the way the method accepts its parameters so that you instead accepted an array. Then you could loop or walk over the array to sanitize its contents. For instance:

public function register( Array $credentials ) {
    array_walk( $credentials, 'mysql_real_escape_string' );

    $this->username = $credentials[ 'username' ];
    //etc...

However, there is a problem with this. For one, these parameters should not be sanitized. Validated: yes; Sanitized: no. You validate for registry; You sanitize for login. If you sanitize them now then you could be changing what that user provided without informing them, which could cause all kinds of confusion. The better thing to do here would be to verify that it has no illegal characters and have the user change it if necessary. Once you validate it, it has essentially been sanitized and you can use it like normal.

Another thing you might consider is using custom sanitizing/validating filters. mysql_real_escape_string() was a very nifty tool when you had nothing better to use, but now there are options such as filter_input() and filter_var(). The former should only be used when accessing raw user input from a form directly, the latter when accepting unknown variables. In this instance, even though you will probably get these values from a form, you will use the latter because your method only sees it as a variable and doesn't know for sure where it comes from.

if( filter_var( $username, FILTER_SANITIZE_STRING ) === $username ) {
    $this->username = $username;
}
if( filter_var( $email, FILTER_VALIDATE_EMAIL ) ) {
    $this->email = $email;
}

You might want to consider adding whitespace for legibility on lines that are too long. The rule of thumb here is 80 characters, including indentation. Both PHP and SQL are very lax when it comes to whitespace, so don't be afraid to use it.

$db->simplequery( "
    INSERT INTO radio_users(
        id,
        username,
        password,
        email
    ) VALUES (
        '',
        '$this->username',
        '$this->password',
        '$this->email'
    )
" );

Now, as to how you can incorporate your $db class into your accountactions class. The easiest, and best, way is to inject it. This can either be done during construction, or via some sort of setter. I'm going to assume that your database model is still MySQL because I can't find anything to contradict it, so I'll use that to type hint it. By the way, you should really upgrade your MySQL to MySQLi or PDO and take advantage of their prepared statements if you haven't already done so.

public function __construct( MySQL $db ) {
    $this->db = $db;
}

/** or as a setter */
public function setDb( MySQL $db ) {
    $this->db = $db;
}

Hope this helps!


Lightweight Angular tooltip component I made a small Popup component to show different error/success...etc. messages.

Template:

<div [@visibilityChanged]="isVisible" *ngIf="shouldShow" [ngClass]="messageType">
    <ng-content></ng-content>
</div>

Component:

import {
  Component, OnChanges, Input,
  Renderer2, AfterViewInit, ElementRef, AfterViewChecked,
  HostListener
} from '@angular/core';

import { trigger, state, animate, transition, style } from '@angular/animations';

/**
 * This class represents the popup component.
 */
@Component({
  moduleId: module.id,
  selector: 'sd-popup',
  templateUrl: 'popup.component.html',
  styleUrls: ['popup.component.css'],
  animations: [trigger('visibilityChanged', [
    transition(':enter', [
      style({ opacity: 0 }),
      animate(100, style({ opacity: 1 }))
    ]),
    transition(':leave', [
      style({ opacity: 1 }),
      animate(100, style({ opacity: 0 }))
    ])
  ])
  ]
})
export class PopUpComponent implements AfterViewInit, AfterViewChecked {
  @Input() messageType: string | string[];
  @Input() errorState: boolean;
  @Input() parentEl: Element;

  public shouldShow: boolean;

  constructor(private popupEl: ElementRef,
    private renderer: Renderer2) {
  }

  ngAfterViewInit(): void {
    // Append to body for better calculation
    document.body.appendChild(this.popupEl.nativeElement);

    //use random delay for load balancing
    this.renderer.listen(this.parentEl, 'keydown', () => {
      setTimeout(() => {
        this.shouldShow = this.errorState;
      }, this.getRandomInt(1, 50));
    });
  }

  @HostListener('window:resize')
  @HostListener('window:scroll')
  ngAfterViewChecked(): void {
    // calculate position when view checked,window is resized or scrolling
    // consider to use load balancing here as well (random delay might be visible)
    this.calculatePosition(this.popupEl.nativeElement, this.parentEl);
  }

  public calculatePosition(element: any, target: any): void {
    const targetOffset = target.getBoundingClientRect();

    this.renderer.setStyle(element, 'top', `${(targetOffset.top + document.documentElement.scrollTop) - element.offsetHeight - 5}px`);
    this.renderer.setStyle(element, 'left', `${targetOffset.left + (target.offsetWidth / 2 - element.offsetWidth / 2)}px`);
    this.renderer.setStyle(element, 'zindex', `${target.zindex + 100}px`);
  }

  getRandomInt(min: number, max: number) {
    return Math.floor(Math.random() * (max - min + 1)) + min;
  }

}

SCSS:

:host {
    position:absolute;
    opacity: 0.9;
 }

 .errorMessage {
    padding: 0.375rem 0.75rem;
    font-size: 1rem;
    line-height: 1.5;
    color: #fff;
    background-image: none;
    background-clip: padding-box;
    border-radius: 0.25rem;
 }

 .warning {
    @extend .errorMessage;
    background-color: #CD464D;
    border: 1px solid #CD464D;
 }

  .success {
     @extend .errorMessage;
     background-color: rgb(15, 230, 147);
     border: 1px solid rgb(15, 230, 147);
  }

 div:after, div:before {
    top: 100%;
    left: 50%;
    border: solid transparent;
    content: " ";
    height: 0;
    width: 0;
    position: absolute;
    pointer-events: none;
}

 div:after {
    border-color: rgba(136, 183, 213, 0);
    border-top-color: inherit;
    border-width: 5px;
    margin-left: -5px;
}
div:before {
    border-color: rgba(194, 225, 245, 0);
    border-top-color: inherit;
    border-width: 6px;
    margin-left: -6px;
}

And finally usage:

<input #passInput type="password" name="password" id="password" class="form-control" formControlName="password"/>
                <sd-popup [parentEl]="passInput" [errorState]="password.errors?.required && (password.dirty || password.touched)"
                    messageType="success">Password is required</sd-popup>

Tell me what do you think about it!

It just looks really good.

I think it stayed this long unanswered because there's not much feedback to give.

I reviewed and liked

The commenting
The naming
The sizes of your functions
Reading flow

I am not convinced that choosing a random int actually helps the load balancing much, but don't have a better alternative. I would also probably put the random number generator in a helper class, it does not really belong in a tooltip component.


PHP mini-password-box: Too many loops

               <?php
                   for($i = 1; $i <= 3; $i++) {
                      $marginRight = ($i < 3 ? "margin-right:4px" : "");
                      echo "
";
                      echo "{$i}. Kind";
                      echo "<select id='param_kind{$i}' class='selFields' name='param_kind{$i}' style='margin-top:3px'>";
                      echo  "<option selected='' value='-1'>--- </option>";
                      for($j = 1; $j <= 16; $j++) {
                         $selected = ($oRecherche->getParamValue("param_kind{$i}") == $j ? "selected='selected'" : "");
                         $option_text = ($j == 1 ? "&lt; 2 Jah." : $j + "Jahre");
                         echo "<option value='{$j}' {$selected}>{$option_text}</option>";
                      }
                      echo "</select>";
                      echo "
";
                   }
               ?>

Two advices:

Use PHP alternative syntax

When mixing HTML and PHP, it is a good practice to use  PHP alternative syntax:

<?php for ($i = 1; $i <= 3; $i++): ?>
..
<?php endfor ?>

Improve the HTML code

Use double quotes in tag attributes (HTML standard)
Try to respect indentation
Move style to css declarations

Final code proposition:

<?php for ($i = 1; $i <= 3; $i++): ?>
  <div <?php echo $i < 3 ? ' class="lt3"' : '' ?>
    <label for="param_kind<?php echo $i ?>"><?php echo $i ?>. Kind
    <select id="param_kind<?php echo $i ?>" class="selFields" name="param_kind<?php echo $i ?>">
      <option value="-1">--- </option>
      <?php for ($j = 1; $j <= 16; $j++): ?>
      <option value=""<?php echo $j ?>"<?php echo $oRecherche->getParamValue("param_kind$i") == $j ? ' selected="selected"' : '' ?>><?php echo $j == 1 ? "&lt; 2 Jah." : $j + "Jahre" ?></option>
      <?php endfor ?>
    </select>

<?php endfor ?>


Algorithm to transform one word to another through valid words I have been practicing backtracking and I wanted to know how I can improve my code. For eg, I don't want to use global. Also, I am not sure if my code will work for all the cases.

# Given two words of equal length that are in a dictionary, write a method to transform one word into another word by changing only
# one letter at a time.  The new word you get in each step must be in the
# dictionary.

# def transform(english_words, start, end):

# transform(english_words, 'damp', 'like')
# ['damp', 'lamp', 'limp', 'lime', 'like']
# ['damp', 'camp', 'came', 'lame', 'lime', 'like']

def is_diff_one(str1, str2):
    if len(str1) != len(str2):
        return False

    count = 0
    for i in range(0, len(str1)):
        if str1[i] != str2[i]:
            count = count + 1

    if count == 1:
        return True

    return False

potential_ans = []

def transform(english_words, start, end, count):
    global potential_ans
    if count == 0:
        count = count + 1
        potential_ans = [start]

    if start == end:
        print potential_ans
        return potential_ans

    for w in english_words:
        if is_diff_one(w, start) and w not in potential_ans:
            potential_ans.append(w)
            transform(english_words, w, end, count)
            potential_ans[:-1]

    return None

english_words = set(['damp', 'camp', 'came', 'lame', 'lime', 'like'])
transform(english_words, 'damp', 'lame', 0)

There are four other answers here, and they all have very sensible advice. But none of them, as far as I can see, points out the terrible runtime complexity of the chosen algorithm, so I think an additional answer is necessary.

1. Bug

>>> english_words = set('bar bat cat war'.split())
>>> transform(english_words, 'bar', 'war', 0)
['bar', 'bat', 'cat', 'war']

As pointed out by Barry, ths is because the line

potential_ans[:-1]

is a mistake for:

potential_ans.pop()

In what follows, I'm going to assume that the bug is fixed.

2. Abstracting the problem

The problem being solved here can be thought of as a problem on a graph, in which each word is a node of the graph, and two words are connected if they differ by one letter. Supposing that the words are cake, came, camp, dame, damp, dike, dime, lake, lame, lamp, like, lime, and limp, then the graph looks like this:

A transformation of one word to another corresponds to a path in this graph. For example one possible transformation of cake to limp corresponds to the highlighted path here:

Finding the shortest transformation between two words corresponds to finding the shortest path between the two nodes in the graph.

There are several benefits of thinking about a problem in terms of abstract data structures like graphs, including:

There are standard terms for describing the problem: here nodes, edges, paths.
There are concrete representations of the data structure that support efficient operations.
You can look up the best known algorithms for solving the problem, instead of having to invent your own.

3. Description and complexity of algorithm

With the bug noted in §1 fixed, what your program does is to generate all the simple paths (paths with no repeated nodes) in the graph of words beginning at start, until it finds a path that leads to end.

But this can take a very very long time:

>>> from timeit import timeit
>>> english_words = set('bar bat cat eat fat hat mat oat pat rat sat tat vat war'.split())
>>> timeit(lambda:transform(english_words, 'bar', 'war', 0), number=1)
['bar', 'war']
2732.44891167

Why does the algorithm take three-quarters of an hour to find this result? Well, the graph being searched looks like this:

The algorithm starts at bar and keeps adding nodes to its path until it can't go any further without repeating a node. For example (depending on the order in which the words are retrieved from the set), it might end up with this path:

After backtracking a couple of steps, it will find this path:

and so on. You'll see that it will explore every path that starts bar—bat and then visits all the —at words. There are \$ 11! = 39,916,800 \$ such paths, and only when it has explored them all will it consider the path bar—war.

Checking to see if two words with \$ m \$ letters differ by one letter takes time \$ O(m) \$, so if there are \$ n \$ words of \$ m \$ letters each, the total runtime is \$ O(n!m) \$.

4. Better algorithm

It is possible to do much better than this if you use one of the standard graph search algorithms such as Dijkstra's or A*. Here's an implementation using the latter.

from collections import defaultdict, namedtuple
from heapq import heappush, heappop

class NotFound(Exception):
    pass

def word_ladder(words, start, end):
    """Return a word ladder (a list of words each of which differs from
    the last by one letter) linking start and end, using the given
    collection of words. Raise NotFound if there is no ladder.

    >>> words = 'card care cold cord core ward warm'.split()
    >>> ' '.join(word_ladder(words, 'cold', 'warm'))
    'cold cord card ward warm'

    """
    # Find the neighbourhood of each word.
    placeholder = object()
    matches = defaultdict(list)
    neighbours = defaultdict(list)
    for word in words:
        for i in range(len(word)):
            pattern = tuple(placeholder if i == j else c
                            for j, c in enumerate(word))
            m = matches[pattern]
            m.append(word)
            neighbours[word].append(m)

    # A* algorithm: see https://en.wikipedia.org/wiki/A*_search_algorithm

    # Admissible estimate of the steps to get from word to end.
    def h_score(word):
        return sum(a != b for a, b in zip(word, end))

    # Closed set: of words visited in the search.
    closed_set = set()

    # Open set: search nodes that have been found but not yet
    # processed. Accompanied by a min-heap of 4-tuples (f-score,
    # g-score, word, previous-node) so that we can efficiently find
    # the node with the smallest f-score.
    Node = namedtuple('Node', 'f g word previous')
    open_set = set([start])
    open_heap = [Node(h_score(start), 0, start, None)]
    while open_heap:
        node = heappop(open_heap)
        if node.word == end:
            result = []
            while node:
                result.append(node.word)
                node = node.previous
            return result[::-1]
        open_set.remove(node.word)
        closed_set.add(node.word)
        g = node.g + 1
        for neighbourhood in neighbours[node.word]:
            for w in neighbourhood:
                if w not in closed_set and w not in open_set:
                    next_node = Node(h_score(w) + g, g, w, node)
                    heappush(open_heap, next_node)
                    open_set.add(w)

    raise NotFound("No ladder from {} to {}".format(start, end))

Suppose that there are \$ n \$ words of \$ m \$ letters, and \$ e \$ edges in the graph of words. Then this algorithm takes \$ O(m^2n) \$ to find the edges and then \$ O(e + mn + n \log n) \$ to find the shortest path.

So this finds shortest ladders on large sets of words in a reasonable amount of time:

>>> dictionary = [w.strip() for w in open('/usr/share/dict/words') if w == w.lower()]
>>> five_letter_words = [w for w in dictionary if len(w) == 5]
>>> len(five_letter_words)
8497
>>> from timeit import timeit
>>> timeit(lambda:print(' '.join(word_ladder(five_letter_words, 'above', 'below'))), number=1)
above amove amoke smoke smoky sooky booky booly bolly bally balli balai balao baloo balow below
0.3188382713124156


Android - handle navigation to multi activities from list view Here is my case I made list of items when I click at any item it should navigate to certain activity.

Here is my I am done First I set number of constants to distinguish between each activity

// constants to show activities onclick event
private static final int LOC = 0, SERVICES = 1, PRE_APPROVE = 2, CLAIMS = 3, BENEFITS = 4, QUE = 5;

Secondly I create method navigate according to this constant

private void ShowActivity(int position) {
        switch (position) {
            case LOC:
                NavigateTo(LocLacatorActivity.class);
                break;
            case SERVICES:
                NavigateTo(ServicesActivity.class);
                break;
            case PRE_APPROVE:
                NavigateTo(PreApproveActivity.class);
                break;
            case CLAIMS:
                NavigateTo(ClaimsActivity.class);
                break;
            case BENEFITS:
                NavigateTo(BenefitsActivity.class);
                break;
            case QUE:
                NavigateTo(QuestionsActivity.class);
                break;
        }
    }

 private void NavigateTo(Class mActivity) {
        Intent intent = new Intent(MainActivity.this, mActivity);
        startActivity(intent);
    }

Finally , here is how I call the method

servicesButton.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                     ShowActivity(SERVICES);
                }
            });

Is this good way to handle multi activity navigation, or can I decrease the size of my methods?

What do you see as the advantage of your approach? Why not just:

servicesButton.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
             showActivity(ServicesActivity.class);
        }
    });

without the constants and the switch converting them to their respective classes? What value do you feel this additional abstraction layer adds to the code?

(If you got rid of it, you'd only need showActivity implemented as follows:

private void showActivity(Class mActivity) {
    Intent intent = new Intent(MainActivity.this, mActivity);
    startActivity(intent);
}

)

A few other remarks:

By convention, names of methods in Java should start with lower-case (so, showActivity and navigateTo - just like startActivity and setOnClickListener).
Parameter names shouldn't start with "m". There is a - controversial - naming convention that prefixes class members (or fields) with "m". I personally think it's iffy, but some people do that. This convention at least has some logic behind it though, whereas prefixing a parameter with "m" makes no sense.
As of now you can pass any class to this method, which isn't as type safe as it could be. If you made it generic, you could narrow it down to Activity subclasses, making it more typesafe:

private <T extends Activity> void showActivity(Class<T> activity) {
    Intent intent = new Intent(MainActivity.this, activity);
    startActivity(intent);
}

Now you can't call eg. showActivity(Integer.class).
Other than removing the "m" prefix from the parameter name, I wouldn't also keep it named "activity". It's an Activity class, not an Activity (instance object) itself. I'd rename it to activityClass.


Build a binary-heap from an array, which method is more efficient and why I am learning about heaps, and I have found two ways of building them from a given array: I am trying to build up a MAX Heap.

1. Top-down approach

Here I check for every element to see if it is at the correct position or not. By using a function called restoreUp, in which every key is compared to its parent key, and if the parent key is smaller than the parent key is moved down. This procedure continues until the parent key is greater. I check it for every key starting at index position 2.

My code is:

void restoreUp(int arr[],int i)
{
    int k=arr[i];
    int par=i/2;
    while(arr[par]<k)
    {
        arr[i]=arr[par];
        i=par;
        par=i/2;
    }
    arr[i]=k;
}
void buildheap1(int arr[],int size)
{
    int i;
    for(i=2;i<=size;i++)
       restoreUp(arr,i);
}

2. Bottom-up approach

Here I start from the first non-leaf node present at index floor(size/2), and call a function restoreDown until node number 1. I compare a key with both its left and right child and then the greater child is moved up. If both children are greater than the key, then I move the larger of the two children up. This procedure stops when both children are smaller than the key.

My code is:

void restoreDown(int arr[],int i,int size)
{
    int left=2*i;
    int right=2*i+1;
    int num=arr[i];
    while(right<=size)
    {
        if(num>arr[left] && num>arr[right])
        {
            arr[i]=num;
            return;
        }
        else if(arr[left]>arr[right])
        {
            arr[i]=arr[left];
            i=left;
        }
        else
        {
            arr[i]=arr[right];
            i=right;
        }
        left=2*i;
        right=2*i+1;
    }
    if(left==size && arr[left]>num)
    {
        arr[i]=arr[left];
        i=left;
    }
    arr[i]=num;
}
void buildheap2(int arr[],int size)
{
    int i;
    for(i=size/2;i>=1;i--)
       restoreDown(arr,i,size);
}

Both the methods are working for me. Which method is more efficient and why?

Someone with more expertise on algorithms may provide a more concrete answer to your specific algorithms. However, I would like to point out two things on a general note:

1. Do the math.

Mathematically determining the complexity of an algorithm usually isn't that hard. Calculate big-O, check number of operations for given inputs and so on. While the answer may not be extremely precise, it's a nice exercise and can be very helpful.

2. Measure twice; cut once.

The best way to figure out the speed of an algorithm is to run it. Calculating the theoretical complexity is useful, but doesn't account for cache misses, branch prediction and the plethora of other things that can affect the execution speed. Run your algorithm on a large number of inputs and test cases to get statistically significant results. Run with and without a profiler to measure what exactly takes time. Sometimes you will find that altering the input, for example by sorting it or changing its layout to reduce cache misses, is more effective than altering the algorithm.


What if any design issues are there in this method of loading configuration data from YAML in Ruby? I am actually pretty excited about this approach, but for sanity's sake I wanted to hear some thoughts on others on my strategy here. My basic goal is to parse a YAML file and recursively create module constants using eval. The core business logic is here:

#
#   configure application based on an easily-managed .yml file -- use 'eval'
#   to generate constants based on the structure of the hash we get from the yaml.
#   most of the business logic for recursively parsing a hash and creating module
#   constants basically just involves tracking module names properly.
#
def parse_and_recursively_create_constants!(hash, module_context = [])
  hash.each do |key, value|
    if value.is_a? Hash
      module_name = convert_string_to_camel_case(key)
      context = module_context.dup << module_name
      parse_and_recursively_create_constants! hash[key], context
    else
      value_is_a_module = false

      if value.is_a? String
        if value.include? "::"
          value_is_a_module = true
        else
          value = "\"#{value}\""
        end
      end

      if value_is_a_module
        # explicitly build out this module (since it doesn't exist) so that we can talk about it
        expression = value.split('::').map { |module_name| "module #{module_name}; "}.join(' ')
        value.split('::').count.times { expression << "end; "}
        eval(expression, $__global_scope)
      end

      # define the given constant
      depth = module_context.count
      expr = module_context.map { |module_name| "module #{module_name}; " }.join(" ")
      expr << "#{key.upcase} = #{value}; "
      depth.times { expr << "end; "}
      expr = "module #{@project_name}; #{expr}; end"
      eval(expr, $__global_scope)
    end
  end

Note I am having to bind the default scope to a global $__global_scope which occurs just before the class definition containing this method. I was wondering if there were some cleaner way of handling that, but either way the behavior certainly is what I want it to be. Here's an example configuration file:

key: value
numeric_data: 3.221
sample_module:
    some_key: another value
key_should_be_module: I::Am::A::Module

And the associated test case:

configure! "Sample", "spec/resources/sample.yml"

Sample::KEY.should == 'value'
Sample::NUMERIC_DATA.should == 3.221
Sample::SampleModule::SOME_KEY.should == 'another value'
Sample::KEY_SHOULD_BE_MODULE.should be_a_kind_of(Module)
Sample::KEY_SHOULD_BE_MODULE.should == I::Am::A::Module

I have released this as a gem, and if you'd like more context the full source is on Github here.

You use eval, but eval is evil.

I tried a solution without eval. For this I used:

Module.const_set to define constants
Module.const_defined to check if a constant is defined
Module.const_get to navigate inside modules
Module.new to define new modules.

My solution (including a little test):

def def_constants!(hash, context)

  #context should be a module. If not, then get (or build) it.
  if context.is_a? String
      lcontext = Object
      context.split('::').each{ |module_name|
        lcontext.const_set(module_name, Module.new) unless lcontext.const_defined?(module_name)
        lcontext = lcontext.const_get(module_name)
      }
    context = lcontext
  end

  raise ArgumentError unless context.is_a?(Module)

  hash.each do |key, value|
    case value
      when Hash
        module_name = key.upcase #fixme --  convert_string_to_camel_case(key)
        def_constants!( hash[key], [context.name, module_name].join('::'))
      when /::/  # explicitly build out this module (since it doesn't exist) so that we can talk about it
        context.const_set(key.upcase, Module.new)
        lmod = context.const_get(key.upcase)
        value.split('::').each{ |module_name|
          lmod = lmod.const_set(module_name, Module.new )
        }
      else
        # define the given constant
        context.const_set(key.upcase, value)
    end
  end #hash
end

module AA #define a start
end

require 'yaml'
def_constants!(YAML.load(DATA), AA)
p AA.constants
p AA::KEY
p AA::SAMPLE_MODULE::SOME_KEY
p AA::KEY_SHOULD_BE_MODULE
p AA::KEY_SHOULD_BE_MODULE::I
p AA::KEY_SHOULD_BE_MODULE::I.constants
p AA::KEY_SHOULD_BE_MODULE::I::Am
p AA::KEY_SHOULD_BE_MODULE::I::Am::A
p AA::KEY_SHOULD_BE_MODULE::I::Am::A::Module

__END__
key: value
numeric_data: 3.221
sample_module:
    some_key: another value
key_should_be_module: I::Am::A::Module

I haven't tested it for each usecase. I expect problems, when you try to define modules, where you have already constants with the same name.

Some changes, to use it in your code:

I skipped convert_string_to_camel_case(key) and replaced it with upcase
Your @project_name is my initial context.
(and sure: I renamed the method ;-) )


Most popular element with count in Array with Java Here is the input array of int values

Example: { 5, 4, 6, 5, 1, 0, 7, 7, 3, 5 }

Find the most repeated element with its count, example output;  5 - repeated 3 times

Here is my code after some improvisations to improve the complexity from O(n)

class RepeatedEntry implements Comparable<RepeatedEntry> {
    private int number;
    private int count;

    public void setNumber(int number) {
        this.number = number;
    }

    public void setCount(int count) {
        this.count = count;
    }

    public int getNumber() {
        return number;
    }

    public int getCount() {
        return count;
    }

    @Override
    public int compareTo(RepeatedEntry obj) {
        return obj.count - this.count;
    }
}

public class MaxCountOfDuplicate {

    public static RepeatedEntry findMostPopularItem(List<Integer> inputList) {
        Collections.sort(inputList);
        Set<RepeatedEntry> resultSet = new TreeSet<>();
        for (int i = 0; i < inputList.size(); i++) {
            int thisEle = inputList.get(i);
            int lastIndexOfThisEle = inputList.lastIndexOf(thisEle);
            int repeatedCount = (lastIndexOfThisEle - i) + 1;
            if (repeatedCount != 1) {
                RepeatedEntry repeatedEntry = new RepeatedEntry();
                repeatedEntry.setNumber(thisEle);
                repeatedEntry.setCount(repeatedCount);
                resultSet.add(repeatedEntry);
            }
        }
        return resultSet.iterator().next();
    }

    /* Given an input array find the duplicate element with max count. */
    public static void main(String[] args) {

        Integer[] x = { 5, 4, 6, 5, 1, 0, 7, 7, 3, 5 }; // input array
        RepeatedEntry finalResult = findMostPopularItem(Arrays.asList(x));

        System.out.println("Most repeated element and its count is -> \n");
        System.out.println(finalResult.getNumber() + " - repeated "
                + finalResult.getCount() + " times");

    }
}

Here are my relevant questions to this

I am sure the time complexity is not O(n) and O(n^2)
In this case, how do I calculate time complexity and what is it in this case?
And coming to space complexity, creating RepeatedEntry objects in a loop, is it a performance issue?

I have assumed the input array to be Integer wrapper, if it was strictly int primitive, the conversions and complexity's would increase I guess. I thought of storing the result in HashMap but avoided as it needs further processing to fetch the output, any answer using HashMap improving complexity would help me learn too and best possible solutions as well.

(Useful: What is the difference between O, Ω, and Θ?)

  How do I calculate time complexity and what is it in this case?

Algorithmic time complexity, as we commonly understand it, is a measure of how an algorithm scales in terms of its input. We're usually interested in asymptotic complexity, meaning we want to see how it performs in the generalized, large-scale cases. It is the answer to the question: "What if we make it bigger?"

How you calculate is fairly straightforward: you figure out the number of steps your algorithm needs compared to your input size, and you crib out the scalars:

Θ(4n² + 25n + 842) = Θ(n²) ; Θ(28n * 6 log (18n)) = Θ(n log n)

... with the understanding that, as n approaches very large numbers, all scalars become insignificant. (This isn't entirely accurate, but it's good enough for envelope-and-fingers calculations.)

Here is a line-per-line of your code:

// annotated with time complexity, with n = inputList.size()
public static RepeatedEntry findMostPopularItem(List<Integer> inputList) {
L1: Collections.sort(inputList); // Θ(n log n) hopefully
L2: Set<RepeatedEntry> resultSet = new TreeSet<>(); // Θ(1)
L3: for (int i = 0; i < inputList.size(); i++) { // Θ(n)
L4:     int thisEle = inputList.get(i); // Θ(1)
L5:     int lastIndexOfThisEle = inputList.lastIndexOf(thisEle); // Θ(n)
L6:     int repeatedCount = (lastIndexOfThisEle - i) + 1; // Θ(1)
L7:     if (repeatedCount != 1) { // Θ(1)
L8:         RepeatedEntry repeatedEntry = new RepeatedEntry(); // Θ(1)
L9:         repeatedEntry.setNumber(thisEle); // Θ(1)
L10:        repeatedEntry.setCount(repeatedCount); // Θ(1)
L11:        resultSet.add(repeatedEntry); // Θ(log n)
L12:    }
L13:}
L14:return resultSet.iterator().next(); // Θ(log n)
}

L3 * L5 gives you an algorithmic time complexity approached by Θ(n²) (consider the case where no numbers are repeated).

If the input is already sorted, you can find the top in a Θ(n) fashion:

E maxItem = null;
int maxLength = 0;

int start, end;
for ( start = 0, end = 1; end < inputList.size(); end++ ) { // Θ(n)
  if ( !inputList.get(end).equals(inputList.get(start)) ) { // Θ(1)
    if ( end - start > maxLength ) { // Θ(1)
      maxItem = inputList.get(start); // Θ(1)
      maxLength = end - start; // Θ(1)
    }
    start = end; // Θ(1)
  }
}
// case: maxItem is top/last element
if ( end - start > maxLength ) { // Θ(1)
  maxItem = inputList.get(start); // Θ(1)
  maxLength = end - start; // Θ(1)
}
return new RepeatedEntry(maxItem, maxLength); // Θ(1)

... but sorting itself is a Θ(n log n) operation in the ideal case.

  I thought of storing the result in HashMap but avoided as it needs further processing to fetch the output, any answer using HashMap improving complexity would help me learn too and best possible solutions as well.

Map<E, Integer> frequency = new HashMap<>();
E maxItem = null;
int maxLength = 0;

for ( E item : inputList ) { // Θ(n)
  int count = frequency.merge(item, 1, Integer::sum); // Θ(1) [!]
  if ( count > maxLength ) { // Θ(1)
    maxItem = item; // Θ(1)
    maxLength = count; // Θ(1)
  }
}
return new RepeatedEntry(maxItem, maxLength); // Θ(1)

The exclamation mark at Map.merge is because, while HashMap provides access in asymptotically constant time, the actual constant time may be significant, and it depends on practical factors like load factor, bucket sizes, and hash spread.


Bakery - Python 3.4.2 I have made a program to calculate the amount of flour, eggs, sugar and butter required for making lemon cakes and cupcakes.

import math

cupcake = [4, 0.1, 12, 14]
#[Butter, egg, flour, sugar]
lemon = [80, 4.5, 240, 300]
#[Butter, egg, flour, sugar]

cup_in = int(input('How many cupcakes would you like? '))
lem_in = int(input('How much lemon cake would you like? '))

amount_cup = []
amount_cup = [x*cup_in for x in cupcake]

amount_lemon = []
amount_lemon = [(x * lem_in) for x in lemon]

print(amount_cup)
print(amount_lemon)

total = []
total = [x + y for x, y in zip(amount_cup, amount_lemon)]

print(total)

amt_butter = total[0]
amt_egg = total[1]
amt_flour = total[2]
amt_sugar = total[3]

bag_but_large = 0
bag_but_medium = 0
bag_but_small = 0

bag_flr_large = 0
bag_flr_medium = 0
bag_flr_small = 0

bag_egg_large = 0
bag_egg_medium = 0
bag_egg_small = 0

bag_sgr_large = 0
bag_sgr_medium = 0
bag_sgr_small = 0

#Calculates the amount of butter bags needed
while amt_butter > 0:

    if amt_butter > 500:
        amt_butter -= 500
        bag_but_large += 1

    elif amt_butter > 250:
        amt_butter -= 250
        bag_but_medium += 1

    elif amt_butter > 125:
        amt_butter -= 125
        bag_but_small += 1

    else:
        bag_but_small += 1
        amt_butter = 0

#Flour
while amt_flour > 0:

    if amt_flour > 750:
        amt_flour -= 750
        bag_flr_large += 1

    elif amt_flour > 500:
        amt_flour -= 500
        bag_flr_medium += 1

    elif amt_flour > 250:
        amt_flour -= 250
        bag_flr_small += 1

    else:
        bag_flr_small += 1
        amt_flour = 0

#Egg
    while amt_egg > 0:

        if amt_egg > 12:
            amt_egg -= 12
            bag_egg_large += 1

        elif amt_egg > 10:
            amt_egg -= 10
            bag_egg_medium += 1

        elif amt_egg > 6:
            amt_egg -= 6
            bag_egg_small += 1

        else:
            bag_egg_small += 1
            amt_egg = 0

#Sugar
while amt_sugar > 0:

        if amt_sugar > 600:
            amt_sugar -= 600
            bag_sgr_large += 1

        elif amt_sugar > 400:
            amt_sugar -= 400
            bag_sgr_medium += 1

        elif amt_sugar > 200:
            amt_sugar -= 200
            bag_sgr_small += 1

        else:
            bag_sgr_small += 1
            amt_sugar = 0

print("*" * 80)
print("Purchases needed: ")
print(bag_but_large, "large bags of butter needed.")
print(bag_but_medium, "medium bags of butter needed.")
print(bag_but_small, "small bags of butter needed.")
print("*" * 80)
print(bag_flr_large, "large bags of flour needed.")
print(bag_flr_medium, "medium bags of flour needed.")
print(bag_flr_small, "small bags of flour needed.")
print("*" * 80)
print(bag_sgr_large, "large bags of sugar needed.")
print(bag_sgr_medium, "medium bags of sugar needed.")
print(bag_sgr_small, "small bags of sugar needed.")
print("*" * 80)
print(bag_egg_large, "large boxes of eggs needed.")
print(bag_egg_medium, "medium boxes of eggs needed.")
print(bag_egg_small, "small boxes of eggs needed.")

You should use functions and better datatypes.

You need to know how many; butter, egg, flour and sugar, bags you need, which come in large, medium and small sizes.
And so wrote the same while loop four times.
Which is perfect for a function!

Instead you should make a function if we keep it simple and have it work the way you have it now,
then you can use a function like trickle_groups(amount: int, large: int, medium: int, small: int) -> tuple<int, int, int>.
Which would get you the function:

def trickle_groups(amount, large, medium, small):
    large_bags, medium_bags, small_bags = 0, 0, 0
    while amount > 0:
        if amount > large:
            amount -= large
            large_bags += 1
        elif amount > medium:
            amount -= medium
            medium_bags += 1
        elif amount > small:
            amount -= small
            small_bags += 1
        else:
            small_bags += 1
            amount = 0
    return (large_bags, medium_bags, small_bags)

We can then use it on all the different ingredients:

butter_bags = trickle_groups(total[0], [500, 250, 125])
egg_bags    = trickle_groups(total[1], [12, 10, 6])
flour_bags  = trickle_groups(total[2], [750, 500, 250])
sugar_bags  = trickle_groups(total[3], [600, 400, 200])

Bam almost all your code is gone!
After this you should want to do the same with your print statements.
Make a format for the three duplicate print lines, and put it in a function.

DISPLAY_FORMAT = '''\
{1} large bags of {0} needed.
{2} medium bags of {0} needed.
{3} small bags of {0} needed.'''

def display_bags(type, bags):
    print(DISPLAY_FORMAT.format(type, *bags))

# ...

print("Purchases needed: ")
display_bags('butter', butter_bags)
display_bags('flour', flour_bags)
display_bags('sugar', sugar_bags)
display_bags('eggs', egg_bags)

After this I'd recommend that you rearrange the way that you go from input to output.
Instead of manually looping, you can instead actually use a for loop.
To do this you need to make a lists to loop through, so ingredients and bag sizes.
After this you'd want to actually loop through them, with zip you can quite simply.
And then you want to put it in a function main, which you call if you're in the main function:

INGREDIENTS = ['butter', 'egg', 'flour', 'sugar']
CUPCAKE = [4, 0.1, 12, 14]
LEMON = [80, 4.5, 240, 300]
BAG_SIZES = [
    [500, 250, 125],
    [12, 10, 6],
    [750, 500, 250],
    [600, 400, 200],
]

def main():
    cup_in = int(input('How many cupcakes would you like? '))
    lem_in = int(input('How much lemon cake would you like? '))

    amount_cup = [x*cup_in for x in CUPCAKE]
    amount_lemon = [x*lem_in for x in LEMON]
    total = [x + y for x, y in zip(amount_cup, amount_lemon)]
    bags = [trickle_groups(a, b) for a, b in zip(total, BAG_SIZES)]
    print("Purchases needed: ")
    for ingredient, bag in zip(INGREDIENTS, bags):
        display_bags(ingredient, bags)

if __name__ == '__main__':
    main()

You can also optimize trickle_groups, rather than looping until the amount is zero, you can instead find the amount of large bags you need, and the remainder with divmod.
Where you get the amount you need with the division, and you get the amount left with the modulo part.
I change it also to take any amount of bags, and so you should be able to get:

def trickle_groups(amount, bag_sizes):
    bag_sizes = list(bag_sizes)
    bag_sizes.sort(reverse=True)
    amount_bags = []
    for bag in bag_sizes:
        n, amount = divmod(amount, bag)
        amount_bags.append(n)
    if amount:
        amount_bags[-1] += 1
    return tuple(amount_bags)


changing values in a vector I am looking to have a code review on this tiny repository.

The code seems to work, but I am not sure if I have written it in idiomatic Rust.

#[derive(Debug)]
pub struct QuickFind {
    items: Vec<u64>,
}

impl QuickFind {
    pub fn new(length: u64) -> QuickFind {
        let mut result: QuickFind = QuickFind { items: Vec::new() };

        for i in 0..length {
            result.items.push(i);
        }

        result
    }

    pub fn union(&mut self, left_index: u64, right_index: u64) {
        let left_group_id = self.items[left_index as usize];

        self.items[right_index as usize] = left_group_id;
    }

    pub fn get_items(&self) -> &[u64] {
        &self.items
    }

    pub fn is_connected(&self, left_index: u64, right_index: u64) -> bool {
        self.items[left_index as usize] == self.items[right_index as usize]
    }
}

Collect the iterator into the new vector

You don't need to loop over a range to populate a vector with elements. The collect method has enough information to construct a Vec<u64> on the spot. This also makes the result variable unnecessary.

    pub fn new(length: u64) -> QuickFind {
        QuickFind {
            items: (0..length).collect(),
        }
    }

C-GETTER

The convention for getter names is without the prefix get_, unless get makes up the entire method name. get_items(&self) becomes items(&self).

Reconsider usize as the index type.

There appear to be multiple conversions from u64 to usize in the code. Indeed, usize is the common integral type for referring to a relative (non-negative) position of a value in memory. With that done, the only integer cast is in new, so as to turn the range (0..length) into a range of values rather than a range of indices.

The final code:

Playground

#[derive(Debug)]
pub struct QuickFind {
    items: Vec<u64>,
}

impl QuickFind {
    pub fn new(length: usize) -> QuickFind {
        QuickFind {
            items: (0..length as u64).collect(),
        }
    }

    pub fn union(&mut self, left_index: usize, right_index: usize) {
        let left_group_id = self.items[left_index];

        self.items[right_index] = left_group_id;
    }

    pub fn items(&self) -> &[u64] {
        &self.items
    }

    pub fn is_connected(&self, left_index: usize, right_index: usize) -> bool {
        self.items[left_index] == self.items[right_index]
    }
}