C++ Class for optionally constructing arbitrary derived classes Here is a class I'm working up to register classes derived from a Module base class. These classes may or may not be instantiated (via the ModuleCreatorBase::create() method) based on some logic which is not yet included in the code.

I wanted to allow constructing Modules which take no arguments and Modules which take any number of arguments, though I'm not sure if my method here is the best approach for that. For some reason it feels kind of clunky to me.

Are there more effective or perhaps other more well established ways to achieve this?

#include <memory>
#include <unordered_map>
#include "Module.hpp"

class ModuleManager
{
public:
  template <typename ModuleClass>
  static void registerModule(const std::string& moduleName);
  template <typename ModuleClass, typename... Args>
  static void registerModule(const std::string& moduleName, Args&&... args);
private:
  struct ModuleCreatorBase
  {
    virtual std::unique_ptr<Module> create() = 0;
  };
  template <typename ModuleClass>
  struct ModuleCreator : public ModuleCreatorBase
  {
    std::unique_ptr<Module> create() override;
  };
  template <typename ModuleClass, typename... Args>
  struct ModuleCreatorArgs : public ModuleCreatorBase
  {
    ModuleCreatorArgs(Args&&... args);
    std::tuple<Args...> args;
    template<std::size_t ...I>
    std::unique_ptr<Module> do_create(std::index_sequence<I...>);
    std::unique_ptr<Module> create() override;
  };
  static std::unordered_map<std::string, ModuleCreatorBase*> registry;
};

template <typename ModuleClass>
std::unique_ptr<Module> ModuleManager::ModuleCreator<ModuleClass>::create()
{
  return std::make_unique<ModuleClass>();
}

template <typename ModuleClass, typename... Args>
ModuleManager::ModuleCreatorArgs<ModuleClass, Args...>::ModuleCreatorArgs(Args&&... args)
: args(std::forward<Args>(args)...) {}

template <typename ModuleClass, typename... Args>
std::unique_ptr<Module> ModuleManager::ModuleCreatorArgs<ModuleClass, Args...>::create()
{
  return do_create(std::index_sequence_for<Args...>{});
}

template <typename ModuleClass, typename... Args>
template <std::size_t ...I>
std::unique_ptr<Module> ModuleManager::ModuleCreatorArgs<ModuleClass, Args...>::do_create(std::index_sequence<I...>)
{
  return std::make_unique<ModuleClass>(std::get<I>(args)...);
}

template <typename ModuleClass>
void ModuleManager::registerModule(const std::string& moduleName)
{
  registry.insert(std::pair<std::string, ModuleCreatorBase*>(moduleName, new ModuleCreator<ModuleClass>()));
}
template <typename ModuleClass, typename... Args>
void ModuleManager::registerModule(const std::string& moduleName, Args&&... args)
{
  registry.insert(std::pair<std::string, ModuleCreatorBase*>(moduleName, new ModuleCreatorArgs<ModuleClass, Args...>(std::forward<Args>(args)...)));
}

Unconstrained templates
The public facing member function templates registerModule never validate any of the types they take. Currently, trying to call registerModule for a type not derived from Module fails due to the instantiation of ModuleCreator::create (and subsequent missing conversion from ModuleClass to Module), this relies on an implementation detail (which might change).
Similarly, the overload of registerModule which takes constructor arguments never validates whether ModuleClass can actually be constructed with them.
Better be explicit about which types are allowed to be passed!
In C++17, one could use a simple if constexpr, but since this question is tagged c++14, using SFINAE is the only option.
Even better, with slightly more work you can give much better error messages than what the compiler generates, so a user of the class doesn't have to parse dozens of lines of template instantiations.
Example using SFINAE (just member function declaration), assuming below fix for ModuleCreatorArgs::args:
template<typename ModuleClass>
static std::enable_if_t<
    std::is_base_of<Module, ModuleClass>::value, void> &&
    std::is_default_constructible<ModuleClass>::value,
void> registerModule(const std::string&);

template<typename ModuleClass, typename... Args>
static std::enable_if_t<
    std::is_base_of<Module, ModuleClass>::value &&
    std::is_constructible<ModuleClass, std::remove_reference_t<Args>...>::value,
void> registerModule(const std::string&, Args&&...);

For error messages, you could add:
template<typename ModuleClass, typename...Args>
static std::enable_if_t<
    !std::is_base_of<Module, ModuleClass>::value ||
    !std::is_constructible<ModuleClass, std::remove_reference_t<Args>...>::value,
void> registerModule(const std::string&, Args&&...)
{
    static_assert(std::is_base_of<Module, ModuleClass>::value, "ModuleClass needs to be derived from Module!");
    static_assert(std::is_constructible<ModuleClass, std::remove_reference_t<Args>...>::value, "ModuleClass is not constructible from the given arguments!");
}

Memory and lifetime management
ModuleManager leaks all created instance of ModuleCreatorBase stored in ModuleManager::registry, as delete is never called. Why not simple use std::unique_ptr<ModuleCreatorBase> instead of ModuleCreatorBase*? (Note: adding calls to delete in the destructor of ModuleManager doesn't fix this problem!)
A bit subtler is the dangling reference issue with ModuleCreatorArgs: Since all the passed arguments are perfectly forwarded to be stored in ModuleCreatorArgs::args, lvalue references (to possibly local values!) will be stored as references, which might dangle when they are actually used for Module construction.
Now, I don't know the requirements for Modules derived classes: Do they need to take references in their constructors (e.g. for non-copyable non-movable objects) that for some reason cannot be passed as pointers?

If no, this gets simple: Change the type of ModuleCreatorArgs::args to std::tuple<std::remove_reference_t<Args>...> so every argument is either moved or copied (as appropriate).
If yes, this will require good documentation and discipline! (Honestly, if you can, try to avoid this mess. This is easy to screw up and hard to get right at the callsite to ModuleManager::registerModule.)

Final notes
I really hopes this class (ModuleManager) gets expanded so it can actually be used to construct objects (all it currently does is creating private state). Maybe add a ModuleManager::create(const std::string&) method?
Also, I don't know your actual use case of Modules, but if you expect them to be shared quite often, you might want to add a create_shared (or similarly named) method that returns a std::shared_ptr<Module> instead.


Generating unique alphanumeric ID's of a fixed length I am needing to generate unique id's for users. The 'unique' part is the problem for me.

The 13 character id's which have a format of the following:

0 + random int with range of 1-9 + random letter + random 3 digit number + string 98XX001 For example: 04X90398XX001 --> (0) (4) (X) (903) (98XX001)

This is the code I currently have for generating the ID's:

package generation;

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

public class id {
private static List<String> generatedIDs = new ArrayList<String>();
public static void main(String[] args) {
    generateIDs(1200);
}

public static void generateIDs(int numOfUsers) {
    for (int i = 0; i < numOfUsers; i++) {
        Random r = new Random();
        int first = r.nextInt((9 - 1) + 1) + 1;
        char second = (char) (r.nextInt(26) + 'A');
        int third = r.nextInt((899 - 100) + 100) + 100;

        String id = "0" + first + second + third + "98XX001";
        generatedIDs.add(id);
        System.out.println(id);

    }
  }
}

Like I said, I am pretty clueless on how to make sure that they are unique.

A large part of your id (8 of 13 characters) is actually static.

That leaves you with 260.000 variations (10 (0-9) x 26 (A-Z) x 1000 (0-999)) of ids to assign to your users. After that, you run out of ids. Will that be enough?

In your current code there is not much you can do besides checking wether the id you generated is already in the generatedIDs list and if it is, try again.

Is there a purpose behind generating the ids randomly? Because if they are only used to identify users, it would make more sense to increment them one number at a time and just remember the last id you gave to a user.

User 1:      (0)(0)(A)(000)98XX001
User 2:      (0)(0)(A)(001)98XX001
User 5000:   (0)(0)(E)(999)98XX001
User 80543:  (0)(3)(B)(542)98XX001
User 260000: (0)(9)(Z)(999)98XX001

Here would be the code for that. You hand the function the id you would like to start from and how many ids from that start-id it should generate.

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

public class MainClass
{
  public static void main(String[] args)
  {
    TestClass gen = new TestClass();
    gen.generateIDs("04X99798XX001", 10);
  }
}

public class TestClass
{
  public static Integer letterToInt(char letter) {
    return letter - 'A' + 1;
  }

  public static char intToLetter(int number) {
    return (char) (90 - (26 - number));
  }

  private static List<String> generatedIDs = new ArrayList<String>();

  public static void generateIDs(String lastID, Integer numOfUsers) {
    String first = lastID.substring(1, 2);
    char second = lastID.substring(2, 3).charAt(0);
    String third = lastID.substring(3, 6);

    Integer firstNum = Integer.parseInt(first);
    Integer secondNum = letterToInt(second);
    Integer thirdNum = Integer.parseInt(third);

    for (Integer i = new Integer(0); i < numOfUsers; i++) {
      thirdNum = new Integer(thirdNum.intValue() + 1);

      if (thirdNum > 999) {
        thirdNum = new Integer(0);
        secondNum = new Integer(secondNum.intValue() + 1);
      }

      if (secondNum > 26) {
        secondNum = new Integer(1);
        firstNum = new Integer(firstNum.intValue() + 1);
      }

      if (firstNum > 9) {
        // Ran out of IDs, do something.
        return;
      }

      first = String.valueOf(firstNum);
      second = intToLetter(secondNum);
      third = String.valueOf(thirdNum);

      while (third.length() < 3) {
        third = "0" + third;
      }

      String nextID = "0" + first + intToLetter(secondNum) + String.valueOf(third) + "98XX001";
      generatedIDs.add(nextID);

      System.out.println(nextID);
    }
  }
}

The result looks like this.

04X99898XX001
04X99998XX001
04Y00098XX001
04Y00198XX001
04Y00298XX001
04Y00398XX001
04Y00498XX001
04Y00598XX001
04Y00698XX001
04Y00798XX001


Checking for duplicate materials This is a follow up question of Importing different type of files into Lists. Where the original lists are acquired.

In this script I am processing the obtained lists, and comparing them with each other to determine whether or not a used material has duplicate functions. This using their most commonly known traits, which are: Shader name, Color property and a texture property

I have the feeling my script is far from optimal, and currently takes roughly 120 seconds to compare 1200 materials. Making the rough iteration time per material 0.10 Where 200 of them are unique.

In the future this script will be expanded to process more different details besides the color/name/texture as well, so advice on how to to keep the process more 'open for change' is appreciated as well.

public void InitShaderSorting()
{
    //wipe data so it won't duplicate if button is pressed again
    unique = new List<Material>();
    broken = new List<Material>();
    duplicate = new List<Material>();
    dupof = new List<Material>();

    for (int i = 1; i < TotalMaterial; i++)
    {
        //Filter broken shaders out first so they can never be a unique one
        if (allMat[i].shader.name == "Hidden/InternalErrorShader")
            broken.Add(allMat[i]);
        //If no unique material has been assigned to compare with, assign one first
        else if (unique.Count == 0)
            unique.Add(allMat[i]);
        //If it aint broken, and there is a unique one. Go and compare the shader properties
        else
            CompareShader(allMat[i], allMat[i].shader.name, allMat[i].HasProperty("_Color"), allMat[i].HasProperty("_MainTex"));
    }
}

/// <param name="sn">Shader Name</param>
/// <param name="hc">Has Color Property</param>
/// <param name="ht">Has Texture Property</param>
void CompareShader(Material mat, string sn, bool hc, bool ht)
{
    bool finished = false;

    for (int i = 0; i < unique.Count; i++)
    {
        //Check if shader names are identical, if not continue as it can't be a duplicate
        if (sn != unique[i].shader.name)
        {
            if (i == unique.Count - 1)
            {
                finished = true;
                break;
            }
            else continue;
        }

        //Check if this material even uses a color
        if (hc)
        {
            //If the color is a match and there is no texture to compare, mark as duplicate
            if (unique[i].color == mat.color && !ht)
            {
                duplicate.Add(mat);
                dupof.Add(unique[i]);
                break;
            }
            else
            {
                if (i == unique.Count - 1)
                {
                    finished = true;
                    break;
                }
                else continue;
            }
        }

        if (ht)
        {
            //If the texture is a match, mark as duplicate
            if (unique[i].mainTexture == mat.mainTexture)
            {
                duplicate.Add(mat);
                dupof.Add(unique[i]);
                break;
            }
        }

        if (i == unique.Count - 1) finished = true;
    }

    //If we reached the end without a match, then this material is unique.
    if (finished) unique.Add(mat);
}

Correctness

At first, I'm afraid your code doesn't find all duplicates: If the compared materials don't use a texture or a color, they won't be marked as duplicates. In your code, if (hc) checks if the material has a color. If not, it is jumped over. If the other material has no color either, this could still be a duplicate.

Performance

The performance of your code is \$O(n^2)\$: For each additional item in allMat, the unique collection will be iterated one time. As this collection grows, this will take longer and longer. This can be certainly improved:

What you actually want to do, is "group" your Texture objects by equality. The groups with size == 1 will be your unique textures, the groups with size > 1 will contain your duplicates.

We can do this very easily using LINQ. All we need to do is implement a custom comparer, that knows how to compare two Texture instances:

public class MaterialComparer : IEqualityComparer<Material>
{
    public bool Equals(Material x, Material y)
    {
        if (ReferenceEquals(x, y)) return true;

        if (x.shader.name != y.shader.name) return false;

        var xHasColor = x.HasProperty("_Color");
        var yHasColor = y.HasProperty("_Color");

        if (xHasColor != yHasColor) return false;
        if (xHasColor && x.color != y.color) return false;

        var xHasTexture = x.HasProperty("_MainTex");
        var yHasTexture = y.HasProperty("_MainTex");

        if (xHasTexture != yHasTexture) return false;
        if (xHasTexture && x.mainTexture != y.mainTexture) return false;

        return true;
    }

    public int GetHashCode(Material mat)
    {
        unchecked
        {
            var hasColor = mat.HasProperty("_Color");
            var hasTexture = mat.HasProperty("_MainTex");

            var hashCode = mat.shader.name.GetHashCode();
            hashCode = (hashCode * 397) ^ (hasTexture ? mat.mainTexture.GetHashCode() : 0);
            hashCode = (hashCode * 397) ^ (hasColor ? mat.color.GetHashCode() : 0);

            return hashCode;
        }
    }
}

The important thing here is to get GetHashCode right: This method is supposed to return two integers that MUST be equal when the compared objects are equal.

We can use it like that:

var groups = allMat.GroupBy(m => m, new MaterialComparer())
                .GroupBy(g => g.Count() == 1)
                .ToArray();

var unique = (groups.SingleOrDefault(g => g.Key) ?? Enumerable.Empty<IEnumerable<Material>>())
    .SelectMany(m => m).ToArray();

var duplicates = (groups.SingleOrDefault(g => !g.Key) ?? Enumerable.Empty<IEnumerable<Material>>())
    .SelectMany(m => m).ToArray();

I put this in a Unit Test to measure the performance for 20,000 random materials:

[TestMethod]
public void TestMaterials()
{
    var random = new Random();
    IEnumerable<Material> allMat = from i in Enumerable.Range(0, 20000)
                                let shader = random.Next(100)
                                let color = random.Next(50)
                                let texture = random.Next(50)
                                select new Material(shader.ToString(),
                                                    color == 0 ? null : color.ToString(),
                                                    texture == 0 ? null : texture.ToString());

    var comparator = new YourImplementation(allMat.ToArray());

    var stopwatch = Stopwatch.StartNew();

    comparator.InitShaderSorting();

    Console.WriteLine(stopwatch.Elapsed);
    stopwatch.Restart();

    var groups = allMat.GroupBy(m => m, new MaterialComparer())
                    .GroupBy(g => g.Count() == 1)
                    .ToArray();

    var unique = (groups.SingleOrDefault(g => g.Key) ?? Enumerable.Empty<IEnumerable<Material>>())
        .SelectMany(m => m).ToArray();

    var duplicates = (groups.SingleOrDefault(g => !g.Key) ?? Enumerable.Empty<IEnumerable<Material>>())
        .SelectMany(m => m).ToArray();

    Console.WriteLine(stopwatch.Elapsed);
}

Result:

00:00:14.0328998  // your custom implementation
00:00:00.0712459  // LINQ


Android ListAdapter design advice I want to write an Android ListAdapter to serve data from a database that look like this:

CREATE TABLE note (_id integer primary key autoincrement,
                   content text not null)
CREATE TABLE tag (_id integer primary key autoincrement,
                  name text not null);
CREATE TABLE relation (_id integer primary key autoincrement,
                       noteid integer not null,
                       tagid integer not null,
                       idx integer not null);

I use this database to store notes and their associated tags as an ordered list. One requirement of the ListAdapter is that it must be able to update the ListView contents if the underlying data changes. I can query all the notes in the database with this query:

SELECT note._id AS noteid,
       note.content,
       relation.idx AS tagidx,
       tag.name
FROM note
LEFT JOIN relation ON (relation.noteid = note._id)
LEFT JOIN tag ON (tag._id = relation.tagid)
ORDER BY noteid, tagidx;

Which will give me results looking like this (null values shown for clarity):

0|foo bar baz|0|x
1|hello world|null|null
2|one more nn|0|yy
2|one more nn|1|y

As you can see, a note with more than one tag will be located on more than one row in the result. This means that I can't look at the cursor size to determine the number of notes, I need to traverse the entire Cursor to get a count. I don't want to do that.

The solution I have come up with so far is to use two cursors: one with the query mentioned above and one with a query containing the number of rows in the notes table (select count(*) from notes). In the constructor I call intializeCursors():

private void initializeCursors() {
    notesCursor.moveToFirst();
    countCursor.moveToFirst();
    count = countCursor.getInt(0);
}

I have implemented  getItem() like this:

public Note getItem(int position) {
    // notes is a List of notes that we have already read.
    if (position < notes.size()) {
        return notes.get(position);
    }

    int cursorPosition = notes.size();
    while (cursorPosition <= position) {
        // Creates a note by reading the correct number of rows.
        Note note = NotesDb.noteFrom(notesCursor);
        notes.add(note);
        ++cursorPosition;
    }
    return notes.get(position);
}

The adapter assumes that the cursors are being managed by some Activity that has called startManagingCursor() on them.

So far so good, I guess. The problem is now how to handle the cursor being requeried. Since I have two cursors I need to register listeners for both of them and when I have received onChange() for both of them I can initializeCursors() and notify any listeners registered to my ListAdapter of a change in the its data.

The requery scenario is the actual reason that I needed two cursors in the first place. I have not figured out a way to count the number of notes in a required cursor other than asking the database using another cursor.

This is the best I have so far. I want to check the sanity of this approach with this group. :-) Is this two cursor approach too complicated? Perhaps I have missed some part of the API that solves this nicely for me?

I would suggest using the built-in CursorAdapter together with a SQLiteCursor. You should be able to construct even your more complex query using the SQLiteQueryBuilder.


Beautiful Number Validator Problem

Adapted from this HackerRank problem.

  A numeric string, \$s\$ is beautiful if it can be split into a sequence of two or more positive integers, \$a_1, a_2,...,a_n\$, satisfying the following conditions:

  \$a_i - a_{a-1} = 1\$ for any \$1 \lt i\le n\$ (i.e., each element in the sequence is 1 more than the previous element).
  No \$a_i\$ contains a leading zero.  For example, we can split \$s = 10203\$ into the sequence {\$1, 02, 03\$}, but it is not beautiful because \$02\$ and \$03\$ have leading zeroes.
  The contents of the sequence cannot be rearranged.  For example, we can split \$s = 312\$ into the sequence {\$3, 1, 2\$}, but it is not beautiful because it breaks our first constraint (i.e., \$1 - 3 \ne 1\$).

Instead of printing YES or NO, I just wanted to return a boolean.

Description

The way I thought about implementing a solution was iterating through all the potential sequences that could be formed.

Because sequences are determined by the value of the first integer, by iterating through all the potential first integer values, you can generate all potential solutions.

For example, take the numeric string 123456. Let's say the first integer is 1. Then the next integer in the sequence would be 2 and so on. However, let's say the first integer is actually 12. Then the next integer in the sequence should then be 13. But it's actually 34. Thus, 12 cannot be the first integer in the sequence. Same if we decided that 123 was the first integer in the sequence.

If we exhaust all possibilities for what the first integer could be, then the numeric string cannot be beautiful.

Implementation Notes

In order to test all possible first integer values, we need to generate all prefixes for the input numeric string that are of length 1 to the length of the input string, rounded down.

For each prefix value, add it to a StringBuilder instance. Then, append the next integer value (so the integer after the prefix value, then the integer after, etc.) until the StringBuilder string length is greater than or equal to the input string or the StringBuilder string, up to that point, does not match the input string.

After exiting this loop, check to see if the StringBuilder string matches the input string. If so, then the string is indeed "beautiful".

If not, move on to the next potential prefix / first integer - if these have all been exhausted, then return false.

Implementation

public class BeautifulNumberValidator {
  public static boolean isValidBeautifulNumber(String s) {
    if (s.length() < 2) {
      return false;
    }

    for (int substringSize = 1; substringSize <= s.length() / 2; substringSize++) {
      long value = Long.parseLong(s.substring(0, substringSize));

      if (value == 0) {
        break;
      }

      StringBuilder sb = new StringBuilder();
      sb.append(value);

      while (sb.length() < s.length() && sb.toString().equals(s.substring(0, sb.length()))) {
        sb.append(++value);
      }

      if (sb.toString().equals(s)) {
        return true;
      }
    }

    return false;
  }
}

Remove redundant code

    if (s.length() < 2) {
      return false;
    }

This is unnecessary.  If s.length() is less than two, then 1 <= s.length() / 2 will be false, as 1/2 is 0 in integer math.  So it will drop through to the final return.

Say what you'll do

      if (value == 0) {
        break;
      }

You could write this more readably as

      if (value == 0) {
        return false;
      }

But it's not necessarily so that this would ever be called.  It's also not clear that this is correct behavior.  For example, this would break an octal or hexadecimal integer that is formatted with a leading zero.

Don't repeat work

     while (sb.length() < s.length() && sb.toString().equals(s.substring(0, sb.length()))) {
        sb.append(++value);
      }

This doesn't require a StringBuilder.  Consider

      int i = 0;
      while (i < s.length()) {
        String number = String.valueOf(value);
        if (!s.substring(i, i + number.length()).equals(number)) {
          break;
        }
        i += number.length();
        value++;
      }

Instead of comparing two ever lengthening strings, this compares just the string representation of the current value.

Does this matter?  You choose to parse into a long value.  That only matters if the number is more than ten digits.  Every single nine-digit value will fit into an int.  This suggests that the beautiful number can be at least twenty digits long.

This doesn't quite fit in the logic of your original program, so I rewrote it as a helper method.

    public static boolean isValidBeautifulNumber(String s, long prefix) {
        for (int i = 0; i < s.length(); ) {
            String number = String.valueOf(prefix);
            if (!s.substring(i, i + number.length()).equals(number)) {
                return false;
            }

            i += number.length();
            prefix++;
        }

        return true;
    }

    public static boolean isValidBeautifulNumber(String s) {
        for (int substringSize = 1; substringSize <= s.length() / 2; substringSize++) {
            if (isValidBeautifulNumber(s, Long.parseLong(s.substring(0, substringSize)))) {
                return true;
            }
        }

        return false;
    }

This significantly simplifies the logic.

If each \$a_i\$ matches the original string, we make it through the loop and return true.  If any fail to match, we return false.


Improving SQLITE3 Left Join Performance I occasional write simple yet long Python scripts using Sqlite3 (I'm not proficient with functional or object oriented programming... yet)

Usually my Sqlite3 is blazing fast.  I'm used to looping through lists of SELECT statements with each SELECT taking a split second, and writing tables to text files in HTML markup. But, this is the first time I've done a LEFT JOIN in Sqlite3, and its taking much longer to complete the script than I had thought. I figured a couple of minutes perhaps, but it's taking several hours to run though all the queries.

I added print statements to narrow down that this LEFT JOIN, within a loop, is where the script hangs up for several minutes, instead taking a split second to perform. It's definitely between the SELECT and the .fetchall.

I have a couple of questions:

Is it normal for Sqlite3 to hang on LEFT JOIN (or any other JOIN)?
Did I code something the wrong way?

I need the code to work fast. If the code appears professional that's definitely a bonus.

print "selecting...",
cur.execute("SELECT \"b\".\"Disposals\" AS \"Disposals\", \
                    \"a\".\"ASSET NUMBER\" AS \"Record\", \
                    \"a\".\"DESCRIPTION\" AS \"DESCRIPTION\", \
                    \"a\".\"NAME_2012\" AS \"GIS_NAME_2012\", \
                    \"a\".\"Notes\" AS \"GIS_Notes\", \
                    \"a\".\"Other\" AS \"GIS_Other\", \
                    \"a\".\"QC_place\" AS \"GIS_place\", \
                    \"a\".\"QC_county\" AS \"GIS_county\", \
                    \"a\".\"lon_\" AS \"Longitude\", \
                    \"a\".\"lat_\" AS \"Latitude\" \
FROM \"resultsPoints\" AS \"a\" \
LEFT JOIN \"html_Tables\" AS \"b\" \
  ON (\"a\".\"ASSET NUMBER\" = \"b\".\"Assetnum\") \
WHERE \"a\".\"DEPARTMENT NUMBER\" LIKE ? \
  OR \"a\".\"AGENCY_BoB\" LIKE ? \
GROUP BY \"a\".\"ASSET NUMBER\" \
ORDER BY \"a\".\"ASSET NUMBER\"",(sqlHunt, sqlHuntBoB))

tableData = cur.fetchall()
print "fetching..",

for r, row in enumerate(tableData):
print " - ",
   for c, column in enumerate(row):
       print ".",
       mysheets[bobAgency].write (r+2, c, str(column))

LEFT JOINS are not inherently slow. It really depends on how you set it up. Are there any indexes on b.Assetnum? Having an index on that would probably speed up your joins tremendously. It probably is doing a full table scan on the join right now if you don't have any indexes.


Determine if Class B uses Class A ( Java Reflection) Below is a function that determines if Class B uses Class A.

Currently, it tests for:

Fields
Superclass
Constructors
Methods

private boolean uses(Class<?> b, Class<?> a){
    // Test for Declared field
    for(Field f:b.getDeclaredFields()){
        if(f.getGenericType().equals(a))
            return true;
    }
    // Test for Inherietence
    if(b.getSuperclass().getName().equals(a.getName()))
        return true;

    // Test for constructors
    for(Constructor<?> c: b.getDeclaredConstructors()){
        for(Class<?> p : c.getParameterTypes())
            if(p.getName().equals(a.getName()))
                return true;
    }
    // Test for methods
    for(Method m:b.getDeclaredMethods()){
        for(Class<?> p:m.getParameterTypes())
            if(p.getName().equals(a.getName()))
                return true;
    }

    return false;
}

Is there a better way to write this function?

Is the name of the function terrible?

Are there any bugs in the code?

Is my test holistic, that is, did I test for everything?

Is my test holistic, that is, did I test for everything?

Currently, this is what you're testing for:

Declared fields on the class with getDeclaredFields();
A superclass with getSuperclass();
The parameters of the declared constructors with getDeclaredConstructors();
The parameters of the declared methods with getDeclaredMethods().

This is what you are not testing for:

Inherited fields: they are not returned by the call to getDeclaredFields(), so fields inherited from superclasses are not considered;
The method return type: you are just checking for the type of the parameters but not the return type with getGenericReturnType();
Annotations on the method (getAnnotations()), on the method parameters (getParameterAnnotations()) or on the method return type (getAnnotatedReturnType());
Annotations on the class itself with getAnnotations();
Exceptions declared to be thrown by the methods with getGenericExceptionTypes(), in the same way, the exceptions declared to be thrown by the declared constructors are also not considered;
All the hierarchy of the class: you are only testing for the potential superclass, but not for the potential implemented interfaces or the super-superclasses;
If the given class is an array type, you are not testing its component type with getComponentType().
For the generic types returned by getDeclaredFields() or getGenericReturnType() for example, you can access the actual type argument by casting the type to ParameterizedType and call getActualTypeArguments(). This will return an array of all the type argument of the parameterized type. For example, if a declared field is a Map<String, Integer>, it will return the array [class java.lang.String, class java.lang.Integer].

  Are there any bugs in the code?

Yes, there is a potential bug: getSuperclass can return null:

  If this Class represents either the Object class, an interface, a primitive type, or void, then null is returned

so you need to take care about handling that. Your current code is

if(b.getSuperclass().getName().equals(a.getName()))

which could throw a NullPointerException.

Other comments:

Make sure to always use curly braces, even if it is not strictly needed here. It will make the code less error-prone for the future.
This only uses the Reflection API. Determining whether a given class A truly uses a given class B or not would require going into the byte-code to see if there are any references through chained method calls, import statements or local variables.


Generate all permutations in C I have written code for generating all the permutations in C as explained here. How can the code be optimized? Are there any memory leaks?

int factorial(int n) {
    int fact = 1;
    for(int i = 2; i <= n; i++)
        fact *= i;
    return fact;
}

int main(){
    int len = 4, i, j;
    int **p;
    int **q = (int**) malloc(sizeof(int*));
    q[0] = (int*) malloc(sizeof(int));
    q[0][0] = 1;
    for(i = 0; i < len; i++){
        p = (int**) malloc(sizeof(int*) * factorial(i + 1));
        for(j = 0; j < factorial(i + 1); j++)
            p[j] = (int*) malloc(sizeof(int) * (i + 1));
        int y=0;
        for(j = 0; j < factorial(i); j++){
            for(int k = 0; k < i + 1; k++){
                int added = 0;
                for(int l = 0; l < i + 1; l++){
                    if(k == l){
                        p[y][l] = i + 1;
                        added = 1;
                    }
                    else
                        p[y][l] = q[j][l-added];
                }
                y++;
            }
        }
        for(j = 0; j < factorial(i); j++)
            free(q[j]);
        free(q);
        q = p;
    }
    for(i = 0; i < factorial(len); i++){
        for(j = 0; j < len; j++)
            printf("%d\t", p[i][j]);
        printf("\n");
        free(p[i]);
    }
    free(p);
}

This code is far from readable.

Optional braces should not be considered optional.
Omitting these braces leaves your code prone to future errors.
Besides this, I personally find it makes your code harder to parse.  I'm so used to seeing loop bodies (or if/else) contained within braces, that it becomes easy to mentally skip that block and say "Yup, this is the looping stuff that's happening."  Here, without the brackets, it just looks like your code has indentation problems, and it is distracting.

Single-letter variable names are not acceptable.
The only place that they're even remotely acceptable is as for loop iteration variables, and even this is a purely historical exception to the rule (and I'd argue that i and j are in violation of this exception as they're not declared within the for loop's initialization statement).

Small variable names don't make your code run faster.
Small variable names don't make your code use less memory.
Small variable names don't make your code compile faster.
Small variable names don't make your executable size smaller.
Small variable names do make your code harder to read.

An amazing example of this last point is your code base where we have six different single letter variable names all in the same scope (plus another two in the factorial function).
To the compiler, it doesn't matter what you name all of your variables.  It's smart enough to keep up with all of them.  I'm not that smart.  I'm looking at six variables with completely meaningless names that I'm expected to keep track of.
A variable's name should tell the human reader exactly what it is meant to store.

It's called main.  It's not called everything.
You did manage to move the factorial function out.  But there's a lot of complexity remaining in main that can surely be refactored out into smaller, bite-size functions that can be good names to explain exactly what they're doing.  And that'd definitely help with your code's readability.


Spawning random small cubes in a big invisible cube using Unity C# My goal was to create a big invisible cube and inside that spawn small random cubes.
These small cubes would have:

A random position within the big cube.
A random colour.
A random size.
A random lifetime.
A higher chance to spawn with offset of 0 on y-axis than any other
offset.
Ability to rotate in sync with the big cube.
Ability to fade away as it ages.
To be replaced with new random cubes as they expire.

To achieve this, here is the code I wrote:
SpawnCube.cs
    public GameObject smallCubePrefab;
    public float rateOfSpawn = 1;
    private float nextSpawn = 0;

    // Update is called once per frame
    void Update()
    {
        // Spawn new cubes at specified spawn rate
        if (Time.time > nextSpawn)
        {
            nextSpawn = Time.time + rateOfSpawn;
            StartCoroutine(FadeOutCube());
        }
    }

    public List<GameObject> SpawnSmallCubes()
    {
        // Create an empty list of cubes
        List<GameObject> cubesList = new List<GameObject>();

        // Spawn cube at random position within the big cube's transform position
        Vector3 randPosition = new Vector3(Random.Range(-1f, 1f), 0, Random.Range(-1f, 1f));

        // Generate higher chance (2 chances in 3) of spawning small cubes with offset = 0 on Y-axis
        List<float> randomY = new List<float>() {0, 0, Random.Range(-1f, 1f)};
        randPosition.y = randomY[Random.Range(0, 3)];
        randPosition = transform.TransformPoint(randPosition * .5f);

        // Spawn small cube
        GameObject smallCube = Instantiate(smallCubePrefab, randPosition, transform.rotation);

        // Give random color
        smallCube.GetComponent<Renderer>().material.color = Random.ColorHSV(0f, 1f, 1f, 1f, 0.5f, 1f);

        // Give random size
        int randSize = Random.Range(1, 10);
        smallCube.transform.localScale = new Vector3(randSize, randSize, randSize);

        // Add spawned cube to the list of cubes
        cubesList.Add(smallCube);

        return cubesList;
    }

    public IEnumerator FadeOutCube()
    {
        // Give random lifetime
        float fadeSpeed = Random.Range(0.01f, 0.05f);

        List<GameObject> smallCubes = SpawnSmallCubes();

        foreach (GameObject cube in smallCubes.ToArray())
        {
            while (cube.GetComponent<Renderer>().material.color.a > 0)
            {
                Color cubeColor = cube.GetComponent<Renderer>().material.color;
                float fadeAmount = cubeColor.a - (fadeSpeed * Time.deltaTime);

                cubeColor = new Color(cubeColor.r, cubeColor.g, cubeColor.b, fadeAmount);
                cube.GetComponent<Renderer>().material.color = cubeColor;
                yield return null;
            }
            if(cube.GetComponent<Renderer>().material.color.a <= 0)
            {
                Destroy(cube);
                smallCubes.Remove(cube);
            }
        }
    }

Rotate.cs
public Vector3 rotationSpeed;

    // Update is called once per frame
    void Update()
    {
        transform.Rotate(rotationSpeed * Time.deltaTime);
    }

Any suggestions for improvement or changes to make it better in any way?
Thanks

Why does SpawnSmallCubes() generate only a single cube?
if(cube.GetComponent<Renderer>().material.color.a <= 0) this line is unecessary
GetComponent() calls can be reduced by holding onto the Renderer instance, assuming you aren't swapping it out
smallCubes.ToArray() conversion is unecessary
FadeOutCube() also spawns the cubes. This isn't obvious from the naming


Retrieving multiple versions of source code for a file I have the following code that checks for three different properties. Unfortunately each call can throw up to four exceptions that I all have to catch. Can I some how make the code more readable by reducing the amount of try/catch?

public void retrieveSourceCode() {
        try {
            fileName = RestServices.getInstance().getClassName(getSourceCodeURI());
        } catch (ClientProtocolException e) {
            fileName = "not available";
            e.printStackTrace();
        } catch (IOException e) {
            fileName = "not available";
            e.printStackTrace();
        } catch (RestServicesException e) {
            fileName = "not available";
            e.printStackTrace();
        }

        if (!(fileName.endsWith(".jar") || fileName.endsWith(".svn-base"))) {
            try {
                sourceCode = RestServices.getInstance().sendGetRequestJsonTextToString(getSourceCodeURI());
            } catch (ClientProtocolException e) {
                sourceCode = "not available";
                e.printStackTrace();
            } catch (IOException e) {
                sourceCode = "not available";
                e.printStackTrace();
            } catch (RestServicesException e) {
                sourceCode = "not available";
                e.printStackTrace();
            }

            if (before == null) {
                beforeSourceCode = "no before source code available";
            } else {
                try {
                    beforeSourceCode = RestServices.getInstance().sendGetRequestJsonTextToString(getBeforeVersionURI());
                } catch (ClientProtocolException e) {
                    beforeSourceCode = "no before source code available";
                    e.printStackTrace();
                } catch (IOException e) {
                    beforeSourceCode = "no before source code available";
                    e.printStackTrace();
                } catch (RestServicesException e) {
                    beforeSourceCode = "no before source code available";
                    e.printStackTrace();
                }
            }

            if (after == null) {
                afterSourceCode = "no after source code available";
            } else {
                try {
                    afterSourceCode = RestServices.getInstance().sendGetRequestJsonTextToString(getAfterVersionURI());
                } catch (ClientProtocolException e) {
                    afterSourceCode = "no after source code available";
                    e.printStackTrace();
                } catch (IOException e) {
                    afterSourceCode = "no after source code available";
                    e.printStackTrace();
                } catch (RestServicesException e) {
                    afterSourceCode = "no after source code available";
                    e.printStackTrace();
                }
            }
        }

        getChangeSet().addAffectedFile(getFileName());

    }

If you upgrade to Java 7, you can use the new Multi-catch syntax. You could change the first block to:

    try {
        fileName = RestServices.getInstance().getClassName(getSourceCodeURI());
    } catch (ClientProtocolException | IOException | RestServicesException e) {
        fileName = "not available";
        e.printStackTrace();
    }

Essentially if there are many exceptions you catch and handle in the same way, you can use multi-catch


Refactor foreach statement to LINQ I have been working on a project and I'm actually refactoring some code. I have encountered myself with lots of foreach and if statements, which could be easily replace with LINQ.

But I have this code snippet, that I wonder how I could make it more functional style.

foreach (var notification in notifications)
{
    if (_emailService.SendEmail(notification.Message.Subject, notification.Message.Body, notification.Message.MailTo))
    {
        successNotificationIDs.AddRange(notification.ID);
    }
    else
    {
        errorCount++;
    }
}

The SendEmail method of the EmailService returns a bool. If its execution has been successfully, it will add an IEnumerable of Int to a declared collection (successNotificationsIDs). If not, I will increase the errorCount variable.

LINQ isn't a silver bullet. It stands for Language-INtegrated-Query, which allows querying objects.

Querying objects isn't something that's supposed to have side-effects. However this is precisely what you loop's body is doing.

Therefore, refactoring it to use LINQ, if at all possible, would make it much less readable than the foreach loop you have here.

You could always create a Task object for each notification, and then run that.

Now the loop body you have, is using AddRange to add what appears to be a single value. Unless notification.ID is an IEnumerable<whatever>, you should be using the Add method to add a single value to your successNotificationIDs.


Plotting a parabola using Tkinter I'm a beginner, and I have just written this basic code that simply plots a parabola in a graph.
Is it possible to make my code more efficient?

The process works by the creating lists of X and Y then adding range of numbers squared to the list Y and normal X to its list and then the function plot simply goes ahead and creates a range doubled + 2 the size ( because the previous range(-size, size) creates both the minus and positive values and a zero) and fills the canvas.create_line(starting X location, starting Y location, ending X location, ending Y location) statement and also creates a log.

import tkinter
from tkinter import Tk

# Adding a Tk window
root: Tk = tkinter.Tk()
root.geometry('640x480')   # Configuring the resolution

# Adding the canvas to put the graph on it
CanvasNo1 = tkinter.Canvas(root, width=640, height=480)
CanvasNo1.grid()

def draw_axis(canvas_object):   # a function in order to draw the horizontal and vertical lines and setting the scroll
    canvas_object.update()
    x_origin = canvas_object.winfo_width() // 2
    y_origin = canvas_object.winfo_height() // 2
    canvas_object.configure(scrollregion=(-x_origin, -y_origin, x_origin, y_origin))
    canvas_object.create_line(x_origin, 0, -x_origin, 0)
    canvas_object.create_line(0, y_origin, 0, -y_origin)

def parabola(number, size):  # Calculating the parabola
    result = number * number // size
    return result

def plot(canvas_object, size, color):  # The grand function to do the plotting process
    y_location = []
    x_location = []
    for each in range(-size, size+1):
        y_location.append(parabola(each, size))
        x_location.append(each)
    for each_of in range(0, size*2+2):
        if each_of <= size*2-1:
            canvas_object.create_line(x_location[each_of], -y_location[each_of], x_location[each_of+1],
                                      -y_location[each_of+1], fill=str(color))
            print('A line from X location of {} and Y location of {} to the X location of {} and Y location of {} was'
                  ' drawn, Color = {}'.format(x_location[each_of], y_location[each_of], x_location[each_of+1],
                                              y_location[each_of+1], color))
        else:
            break

draw_axis(CanvasNo1)
plot(CanvasNo1, 500, 'red')
CanvasNo1.mainloop()   # running the window

Instead of creating an empty list and appending the values to it, try using list comprehension.

so instead of

y_location = []
x_location = []
for each in range(-size, size+1):
    y_location.append(parabola(each, size))
    x_location.append(each)

try

y_location = [parabola(each,size) for each in range(-size,size+1)]
x_location = list(range(-size,size+1))

In general you can use a profiler to see which part of your code requires the largest amount of computing making it easier to improve the code.
https://docs.python.org/2/library/profile.html


White Elephant Gift Exchange game using object-oriented design I'm learning about OOP and I'm attempting to solve the White Elephant Gift Exchange game. The way I've done it is that I have created a bunch of set variables which store the set of gifts opened, set of gifts not opened yet, set of users. I was wondering if this is a good way of designing the classes and delegating work.

Could someone please comment on how can the design be improved and how could I make it more object-oriented?

Game rules

  In its most basic form, the game is as follows: each participant supplies one wrapped gift. The gifts are placed in a central location, and participants determine in what order they will take turns selecting them. The first person opens a wrapped gift and the turn ends. On subsequent turns, each person can open a new present or gets the choice to "steal" another person's unwrapped gift. When a person's gift is stolen, that person can either choose another wrapped gift to open or can steal from another player. The game is over when the last person goes and the first person goes again (but see variations below).

User.java

class User {
    private Gift giftBrought;
    private Gift giftWon;

    public Gift pickOrSteal(Set<Gift> unopnenedGifts, Set<Gift> openedGifts) {}
}

Gift.java

class Gift {
    private boolean isOpened;
    private User owner;
}

WhiteElephant.java

class WhiteElephant {
    private List<User> users;
    private Set<User> turnCompletedUsers;
    private Set<Gift> unopnenedGifts;
    private Map<Gift, User> openedGifts;
    private Set<Gift> unavailableForRoundGifts;

    public void assignRanks() {
        // use knuth shuffle to randomize user array
    }

    public List<Gift> getUnopenedGifts() {}
    public List<Gift> getOpenedGifts() {}

    public boolean isGiftAvailableForStealing(Gift gift) {
        return !unavailableForRoundGifts.contains(gift);
    }

    public Gift pickGift() {}

    public boolean stealGift(User from, User to, Gift gift) {}

    public void play() {
         assignRanks();

         for(int i = 0; i < users.size(); i++) {
             nextUser = users.get(i);
             unavailableForRoundGifts.clear();

             while(nextUser != null) {
                 User currUser = nextUser;
                 Gift gift = currUser.pickOrSteal(unopenedGifts, openedGifts.keyset());
                 if(isGiftAvailableForStealing(gift)) {
                     if(!gift.isOpened()) {
                         unopenedGifts.remove(gift);
                         openedGifts.add(gift);
                         gift.setIsOpened(true);
                         nextUser = null;
                     } else {
                         nextUser = openedGifts.get(gift);
                         stealGift(nextUser, users.get(i), gift);
                     }
                     unavailableForRoundGifts.add(gift);
                     users.get(i).setGiftWon(gift);
                 }
             }
         }
    }
}

Technical issues

In your WhiteElephant class, turnCompletedUsers is unused. Is this expected?
For the purposes of this site*, I think it'll be better if you can actually fill in the implementation for WhiteElephant.stealGift() and WhiteElephant.pickGift() (which happens to be unused, too).
You do not need to make all your methods in WhiteElephant public, probably only play() should remain public and the rest can be private.
openedGifts.add(gift) is actually a syntax error*, as openedGifts is a Map. Perhaps you were looking for openGifts.put(gift, currentUser)?
stealGift() returns a boolean value, but what does that indicate?

Gameplay

  The first person opens a wrapped gift and the turn ends.

Your implementation does not take care of the first player, where the turn ends immediately after opening a gift.

Gift gift = currUser.pickOrSteal(unopenedGifts, openedGifts.keySet());
if (isGiftAvailableForStealing(gift)) {
    ...
}

This two lines seem a bit weird, as I don't think the game rules makes a separate decision concerning whether a gift is available for stealing after a player picks/steals a gift.

An alternative approach

I'll leave this as an exercise for the reader, how about considering adding a new field inside Gift that represents the current holder of the item? In that case, the same isOpened() method will read something like:

public boolean isOpened() {
    return holder != null;
}

The other operations to complete the 'stealing' of gifts, i.e. swapping the players holding the gift, becomes slightly easier, I would think...

* In fact, we only review complete and working code, I am letting these slide because it's not too far a stretch to assume what might be the underlying implementation... but only just.


Cut each N symbol from array What is the most efficient and productive way to cut each N symbol from a string?
For instance,

n = 5;
str = '1234A1234B1234C';
result: "123412341234"

This is my approach:
def delete_each_n(str, n)
  i = n

  str.length/n.times do
    str.slice!(i-1)
    i += (n - 1)
  end

  str
end

Your code

You have a sneaky bug in your code!

1000/2.times {|i| puts i }

You seem to think that this code would display 500 numbers between 0 and 499.
It doesn't. Instead, it displays 0, 1, and returns 500.

You need to replace str.length/n.times do with (str.length/n).times do.

Alternative

You can use gsub to look for the substrings, chop them and replace them:

def delete_every_nth_char(string, n)
  string.gsub(/.{#{n}}/){ |sub| sub.chop }
end

delete_every_nth_char('1234A1234B1234C', 5)
# "123412341234"
delete_every_nth_char('ABAB', 2)
# "AA"
delete_every_nth_char('ABA', 2)
# "AA"
delete_every_nth_char('ABA', 1)
# ""
delete_every_nth_char('ABA', 5)
# "ABA"

It is concise and probably faster than splitting and joining the strings manually.


Custom checksum algorithm in Python How can I make the following checksum algorithm more pythonic and readable?

items = {1:'one', 2:'two', 3:'three'}
text = "foo" # This variable may change on runtime. Added to allow the execution
number = 1 # This variable may change on runtime. Added to allow the execution

string = text + items[number] + " " + "trailing"
if items[number] == "two": string=text
string = [ord(x) for x in string]
iterator = zip(string[1:], string)
checksum = string[0]
for counter, (x, y) in enumerate(iterator):
    if counter % 2:
        checksum += x
        checksum += 2 * y
    else:
        checksum += x * y
checksum %= 0x10000

print("Checksum:", checksum)

The thing called salt by everyone here is fixed and won't change ever. Also, the code is into a bigger function.

This code as-is must return this:

Checksum: 9167

First of all, I would apply the "Extract Method" refactoring method and put the logic behind generating a checksum into a get_checksum() function.

Also, I would avoid hardcoding the trailing "salt" and the 0x10000 "normalizer" and put them into the default keyword argument value and to a "constant" respectively.

You can use sum() instead of having loop with if and else, which increase the overall nestedness. But, it might become less explicit and potentially a bit more difficult to understand.

Comments and docstrings would definitely be needed to describe the logic behind the checksum calculations.

Variable naming can be improved - str_a, str_b and string are not the best choices (even though the part1 and part2 in the code below are not either - think about what these strings represent - may be you'll come up with better variable names). Also, note that you are reusing the string variable to keep the "ords" of each character, which, strictly speaking, is not good - the variable name does not correspond to what it is used for.

At the end you would have something like:

DEFAULT_SALT = "trailing"
NORMALIZER = 0x10000

def get_checksum(part1, part2, salt=DEFAULT_SALT):
    """Returns a checksum of two strings."""

    combined_string = part1 + part2 + " " + salt if part2 != "***" else part1
    ords = [ord(x) for x in combined_string]

    checksum = ords[0]  # initial value

    # TODO: document the logic behind the checksum calculations
    iterator = zip(ords[1:], ords)
    checksum += sum(x + 2 * y if counter % 2 else x * y
                    for counter, (x, y) in enumerate(iterator))
    checksum %= NORMALIZER

    return checksum

We can see that the test you've mentioned in the question passes:

$ ipython3 -i test.py
In [1]: get_checksum("foo", "one")
Out[1]: 9167


The correct place to implode array - MVC and Repositories concept I am writing an application with the repositories concept in php with laravel framework.
In my controller I have this method:

/**
 * Update the specified resource in storage.
 * PUT /locais/{id}
 *
 * @param  int  $id
 * @return Response
 */
public function update($id)
{
    $data = Input::all();
    $data['str_categories'] = implode(',', $data['categories']);
    $this->place->update($id, $data);

    return Redirect::route('locais.index');
}

I am concerned about this line in controller:
$data['str_categories'] = implode(',', $data['categories']);

This line can be in controller method? Or this is a responsibility of places repository?

Logic or data transformation should usually be moved out of the controller.  Your places repository seems like a better choice for the implode.  After all, one of the major purposes of repositories is to decouple data handling and storage as much as possible from other parts of the application, including controller.s


Parse WAP-230 "variable length unsigned integers" I am working on a program to decode an MMS PDU file.  In the file, the "content-length" is represented in a unique way.  According to the MMS spec WAP-230 Section 8.1.2, it is encoded as a "Variable Length Unsigned Integer".

Basically for each byte, the 1st bit is the "continue bit" and the other 7 are the "payload".  We keep reading bytes while the "continue bit" is 1.  When it's 0, we take the "payloads" and combine them together into a value.

Here's an example, let's say I have the following bytes:

82 E5 04

or in binary:

1000 0010 1110 0101 0000 0100

Then we split them into their continue bit/payload:

1 | 0000010
1 | 1100101
0 | 0000100

Now, we start from the beginning, append the bits and move on until the continue bit is 0.  Thus we get the value:

000001011001010000100

or broken into bytes (and left padded with zeroes):

0000 0000 1011 0010 1000 0100

This can be read (in hex) as:

00 B2 84

which converts to 45700 (0xB284).

I tried to implement this in python as I was reading through an MMS PDU file byte by byte.  Here is what I came up with:

cont_bit = True
remaining_bits = []

while cont_bit:
    variable_length = self.data[curr_index]
    curr_index += 1

    # There's obviously a better way to do this, but I don't really know what it is
    binary_length = bin(variable_length).lstrip('0b').zfill(8)

    # Check the "continue bit"
    cont_bit = (binary_length[0] == '1')
    remaining_bits.append(binary_length[1:])

# Put the values together and read it as an int
content_length = int(''.join(remaining_bits), 2)

Note: self.data is the binary file I am reading and curr_index is my current position in the file.

This does work and content_length does contain the right value, I just think there's gotta be a better way to do this than to convert each byte into a string (representing its binary representation), reading the 1st character of this string, then appending the rest of the string into an array (which I parse as a base-2 integer).

To get the bits out of a byte without converting to a string and back again, use Python's bitwise operations and shift operations. To get the high bit of a byte, shift it right by 7 bits:

>>> data = open('/dev/urandom', 'rb').read(128)
>>> data[0]
193
>>> data[0] >> 7
1

To get the low seven bits of a byte, mask against 0b1111111 (that is, 127):

>>> data[0] & 127
65

Finally, if we maintain a running value for content_length, we can extend it with these seven bits of payload by taking the running value, shifting it left by 7 bits and or-ing with the payload. In summary:

content_length = 0
while True:
    byte = self.data[curr_index]
    curr_index += 1
    content_length = (content_length << 7) | (byte & 127)
    if byte >> 7 == 0:
        break


Short Java FizzBuzz using recursion and parameters public void fizzbuzz(int count, int max, String fizzer) {
    fizzer += (count % 3 == 0) ? "fizz":"";
    fizzer += (count % 5 == 0) ? "buzz":"";
    fizzer += (count % 3 != 0 && count % 5 != 0) ? count:"";
    if (count != max) {
        fizzer += "\n";
        fizzbuzz(count+1,max,fizzer);
    } else {
        System.out.println(fizzer);
    }
}

fizzbuzz(1,30,"");

The code is performing fizzbuzz over the range count to max. No variables are initialized within the method.

I'd love to get rid of max, and instead take only two arguments for the method (String fizzer and int count), but how can I? I can do it backwards (i.e. working down until reaching the trivial case count == 0 and then stop), but is there a way to go from 1 to n with only one argument?

Recursion is usually not a favoured strategy in Java due to concerns about stack overflow and function call efficiency.  However, we can still review FizzBuzz as an exercise in recursive programming.

The first observation I have is that recursive solutions usually start by checking for the base case first.  I recommend that you stick to that pattern.  As a bonus, checking for count > max will prevent infinite recursion if the function is accidentally called with the count and max parameters reversed.

Another typical feature of recursive solutions is that they avoid mutation and reassignment.  When you recurse, previous values of your variables will already be kept on the stack for you, and that is generally sufficient to solve most problems.  In your case, you've used +=, whereas you should be able to use just +.

You've used three ternary conditionals.  If you reorder them, you can avoid concatenating += "fizz" followed by += "buzz".

Since this is a pure function, it should probably be declared static, so that you don't have to instantiate an object just to call the code.

Finally, note that the resulting string is already terminated by \n, so perhaps you should call System.out.print() rather than .println().

public void fizzbuzz(int count, int max, String accumulator) {
    // Check for the base case
    if (count > max) {
        System.out.print(accumulator);
        return;
    }

    // ... then proceed to the recursive cases.
    //
    // The first case below would be better written as (count % 15 == 0),
    // but I've kept it as (count % 3 == 0 && count % 5 == 0) for easier
    // comparison with your original code.
    String fizzer = (count % 3 == 0 && count % 5 == 0) ? "fizzbuzz\n" :
                    (count % 3 == 0) ? "fizz\n" :
                    (count % 5 == 0) ? "buzz\n" : count + "\n";
    fizzbuzz(count + 1, max, accumulator + fizzer);
}

You asked about reducing the number of parameters.  The first parameter I would eliminate is not count or max, but the accumulator.  You could change it into a return value instead, and it would be more natural.  The caller would have to be responsible for printing the result — and I would also consider that an improvement, since it keeps the calculation and the output routines separate.

public static String fizzbuzz(int count, int max) {
    if (count > max) {
        return "";
    }
    String fizzer = (count % 15 == 0) ? "fizzbuzz\n" :
                    (count %  3 == 0) ? "fizz\n" :
                    (count %  5 == 0) ? "buzz\n" :
                                        count + "\n";
    return fizzer + fizzbuzz(count + 1, max);
}

public static void main(String[] args) {
    System.out.print(fizzbuzz(1, 30));
}

Once you've done that, you can eliminate one more parameter by making it left-recursive instead of right-recursive.

public static String fizzbuzz(int n) {
    if (n <= 0) {
        return "";
    }
    String fizzer = (n % 15 == 0) ? "fizzbuzz\n" :
                    (n %  3 == 0) ? "fizz\n" :
                    (n %  5 == 0) ? "buzz\n" :
                                    n + "\n";
    return fizzbuzz(n - 1) + fizzer;
}

public static void main(String[] args) {
    System.out.print(fizzbuzz(30));
}


Python implementation of the longest increasing subsequence Prompted by this question on Stack Overflow, I wrote an implementation in Python of the longest increasing subsequence problem.  In a nutshell, the problem is: given a sequence of numbers, remove the fewest possible to obtain an increasing subsequence (the answer is not unique).

Perhaps it is best illustrated by example:

>>> elems
[25, 72, 31, 32, 8, 20, 38, 43, 85, 39, 33, 40, 98, 37, 14]
>>> subsequence(elems)
[25, 31, 32, 38, 39, 40, 98]

The algorithm iterates over the input array, X, while keeping track of the length longest increasing subsequence found so far (L). It also maintains an array M of length L where M[j] = "the index in X of the final element of the best subsequence of length j found so far" where best means the one that ends on the lowest element.

It also maintains an array P which constitutes a linked list of indices in X of the best possible subsequences (e.g. P[j], P[P[j]], P[P[P[j]]] ... is the best subsequence ending with X[j], in reverse order).  P is not needed if only the length of the longest increasing subsequence is needed.

The code below works, but I am sure it could be made shorter and / or more readable. Can any more experienced Python coders offer some suggestions?

from random import randrange
from itertools import islice

def randomSeq(max):
  while True: yield randrange(max)

def randomList(N,max):
  return list(islice(randomSeq(max),N))

## Returns the longest subsequence (non-contiguous) of X that is strictly increasing.
def subsequence(X):
    L = 1     ## length of longest subsequence (initially: just first element)
    M = [0]   ## M[j] = index in X of the final member of the lowest subsequence of length 'j' yet found
    P = [-1]
    for i in range(1,X.__len__()):
        ## Find largest j <= L such that: X[M[j]] < X[i].
        ## X[M[j]] is increasing, so use binary search over j.
        j = -1
        start = 0
        end = L - 1
        going = True
        while going:
            if (start == end):
                if (X[M[start]] < X[i]):
                    j = start
                going = False
            else:
                partition = 1 + ((end - start - 1) / 2)
                if (X[M[start + partition]] < X[i]):
                    start += partition
                    j = start
                else:
                    end = start + partition - 1

        if (j >= 0):
            P.append(M[j])
        else:
            P.append(-1)

        j += 1
        if (j == L):
            M.append(i)
            L += 1
        if (X[i] < X[M[j]]):
            M[j] = i

    ## trace subsequence back to output
    result = []
    trace_idx = M[L-1]
    while (trace_idx >= 0):
        result.append(X[trace_idx])
        trace_idx = P[trace_idx]

    return list(result.__reversed__())

l1 = randomList(15,100)

See the revised version below, in this answer.

from random import randrange
from itertools import islice

def randomSeq(max):

The python style guide recommend that you use space_with_underscores for function names

  while True: yield randrange(max)

def randomList(N,max):
  return list(islice(randomSeq(max),N))

I suspect that's not a really efficient way to produce a random list. I think the following may be a better option:

return [randrange(max) for x in range(N)]

I've not done any benchmarking so I may be wrong.

## Returns the longest subsequence (non-contiguous) of X that is strictly increasing.
def subsequence(X):
    L = 1     ## length of longest subsequence (initially: just first element)
    M = [0]   ## M[j] = index in X of the final member of the lowest subsequence of length 'j' yet found
    P = [-1]

Consider using names instead of these single letter variables.

    for i in range(1,X.__len__()):

Don't use x.__len__() use len(x)

        ## Find largest j <= L such that: X[M[j]] < X[i].
        ## X[M[j]] is increasing, so use binary search over j.

This whole bit about the binary search would do well in a seperate function so as not the clutter the subsequence implementation. Actually python already provides a binary search function in the bisect module.

        j = -1
        start = 0
        end = L - 1
        going = True
        while going:
            if (start == end):

You don't need those parens

                if (X[M[start]] < X[i]):
                    j = start
                going = False

Use break. break isn't pretty when you can avoid it, but setting booleans flags is worse.

            else:
                partition = 1 + ((end - start - 1) / 2)

Notice how you only use start + partition? Calculate the middle element instead. It'll make this whole section a bit neater.

                if (X[M[start + partition]] < X[i]):
                    start += partition
                    j = start
                else:
                    end = start + partition - 1

        if (j >= 0):
            P.append(M[j])
        else:
            P.append(-1)

        j += 1
        if (j == L):
            M.append(i)
            L += 1
        if (X[i] < X[M[j]]):
            M[j] = i

    ## trace subsequence back to output
    result = []
    trace_idx = M[L-1]
    while (trace_idx >= 0):
        result.append(X[trace_idx])
        trace_idx = P[trace_idx]

    return list(result.__reversed__())

use reversed(list)

l1 = randomList(15,100)


Deck and Card classes and member-accessing with one header Previous review of this project:

Card Deck class for a Poker game - version 2

This time, I considered combining both classes into one header file so that I can reduce my use of #include and to keep them together.  I also did this so that I can define my const std::string variables only once for both classes.  However, I decided to try using a namespace for these variables for easier access.  It makes more sense (to me) to keep them with Card and let Deck refer to them during the latter's construction, but I'm not sure how to do that.

Is this the proper thing to do, or could something be done differently?  Is there a need for extern somewhere?  The code below doesn't include class implementations, but I've kept them intact from the linked code.

Deck.h

#ifndef DECK_H
#define DECK_H

#include <iostream>
#include <string>
#include <array>

namespace rs
{
    const std::string RANKS = "A23456789TJQK";
    const std::string SUITS = "HDCS";
}

class Card
{
private:
    unsigned rankValue;
    char rank;
    char suit;

public:
    Card();
    Card(char r, char s);
    bool operator<(const Card &rhs) const {return (rankValue < rhs.rankValue);}
    bool operator>(const Card &rhs) const {return (rankValue > rhs.rankValue);}
    bool operator==(const Card &rhs) const {return (suit == rhs.suit);}
    bool operator!=(const Card &rhs) const {return (suit != rhs.suit);}
    friend std::ostream& operator<<(std::ostream &out, const Card &aCard)
        {return out << '[' << aCard.rank << aCard.suit << ']';}
};

class Deck
{
private:
    static const unsigned MAX_SIZE = 52;
    std::array<Card, MAX_SIZE> cards;
    int topCardPos;

public:
    Deck();
    void shuffle();
    Card deal();
    unsigned size() const {return topCardPos+1;}
    bool empty() const {return topCardPos == -1;}
    friend std::ostream& operator<<(std::ostream &out, const Deck &aDeck);
};

#endif

I see some things that may help you improve your code.

Consider using an enum class

Instead of using constructs such as char suit, use the superior safety of an enum class.  You can read this question for an overview of when to use one, but I think this would be a better way to do things for this case:

enum class suits : char {HEART = 'H', DIAMOND = 'D', CLUB = 'C', SPADE = 'S'};
enum class ranks : char {ACE=1, TWO, THREE, FOUR, FIVE, SIX, SEVEN,
                         EIGHT, NINE, TEN, JACK, QUEEN, KING };

This allows the small storage size of a char, the convenience of an enum and the type-safety of a class.

Force only valid cards to be created

With the existing default constructor, and with the constructor that takes arguments, any enforcement on validity of initialized values becomes part of the detail of the constructor rather than part of the interface.  By using the enum class types mentioned above, we can make it very clear from the interface that it's not going to be possible to create invalid cards:

Card(ranks r, suits s);

Delete unhelpful autogenerated operators

Duplicating cards will get you kicked out of most casinos.  It's probably better to avoid such trouble before it begins by making it a little harder to do:

Card() = delete;
Card(Card&) = delete;

Wrap all of your objects inside a name

Rather than putting just the constants in a namespace, put your whole interface into a namespace.  In some future use in which your code is used within an electronic poker game cruise ship simulator, your Card could then be distinguished from a circuit Card that might be part of the electronic game classes, and your Deck could be distinguished from the Decks of a cruise ship.

Reconsider your class design

The Deck class is really just a collection of Card objects.  While you may have ideas on how to use your Deck class, it's very likely that you're going to want to have many other kinds of Card collections such as Hand or DiscardPile or Shoe (if you're simulating casino blackjack).  In addition, even the notion of Deck can change if you're using it for, say, Pinochle which uses a 48-card deck rather than a 52-card deck.  For that reason, it may be better to define a generic Card collection and then specialize that for the various uses.

Reconsider your comparison operators

The comparison operators for the Card class may well be valid for your particular use, in which case, you should certainly use them, but comparisons and relative valuations tend to change by the particular rules of each card game.  In some games the Ace is low, and in others high.  In some, such as blackjack, it can take on the values of 1 or 11.  For that reason, it is likely that the comparison operators are likely to need to be changed.  This suggests that they could be made virtual so that derived Card classes can override them.


Counts instances for each category for many Matlab files This is for machine learning.  I have 5 matlab files: AWA, REM, S1, S2, SWS.
Each one has 2 arrays: x and y. y are the labels. I want to know the number of instances for each category in each file:

I load the matlab file

I convert from f8 to int32

If I do not use ravel, bincount will not work

I count the instances for each category in the file
I repeat these 4 steps 5 times (as there are 5 files). Is there a way to do it better?

Then I just join all the vectors in one array to send them to PANDAS.

#AWA
awa_y=sio.loadmat('/home/AWA_FeaturesAll.mat')['y']
awa_y=awa_y.astype('int') #from f8 to int32
awa=np.ravel(awa_y) #needed in order to use np.bincount()
AWA=np.bincount(awa)

#Rem
rem_y=sio.loadmat('/home/Rem_FeaturesAll.mat')['y']
rem_y=rem_y.astype('int')
rem=np.ravel(rem_y)
REM=np.bincount(rem)

#S1
s1_y=sio.loadmat('/home/S1_FeaturesAll.mat')['y']
s1_y=s1_y.astype('int')
s1=np.ravel(s1_y)
S1=np.bincount(s1)

#S2
s2_y=sio.loadmat('/home/S2_FeaturesAll.mat')['y']
s2_y=s2_y.astype('int')
s2=np.ravel(s2_y)
S2=np.bincount(s2)

#SWS
sws_y=sio.loadmat('/home/SWS_FeaturesAll.mat')['y']
sws_y=sws_y.astype('int')
sws=np.ravel(sws_y)
SWS=np.bincount(sws)

#Joining the 5 vectors
table= (np.vstack((AWA, REM, S1, S2, SWS))).T
TABLE = np.delete(table, (0), axis=0)
print(TABLE)

You felt it right, there's a lot of unnecessary copy/paste in the first part.

Your 5 data structures are similar, and you create a lot of variables to perform the same processing 5 times.

I did a quick factorisation which brings back the code to a reasonable 12 lines (I could not test, obviously, but that seems OK):

result_list = []

for name in ["AWA","Rem","S1","S2","SWS"]:
    xx_y=sio.loadmat('/home/{}_FeaturesAll.mat'.format(name))['y'].astype('int')
    xx=np.ravel(xx)
    result_list.append(np.bincount(xx))

#Joining the 5 vectors
table= (np.vstack(result_list)).T
TABLE = np.delete(table, (0), axis=0)
print(TABLE)

As the names are logical, I don't have to make a list of full names, only a part is enough. I just apply 5 times the same operation in the correct order, then store in result_list.

Joining the vectors is now a piece of cake. Just pass the result list.

Had you needed the individual vectors further in your code, then a collections.OrderedDict would be better to store them by name and recall them afterwards.


Code conversion from Access to T-SQL I have an Access database that I'm converting to SQL Server 2008. One of the queries in the database uses the LAST function. The table has a AutoNumber ID so true order is simple.

SELECT tbl_unit.unit,
       LAST(tbl_unit.date)     AS Date,
       LAST(tbl_unit.mileage)  AS Mileage
FROM   tbl_unit
GROUP  BY tbl_unit.unit

SQL 2008 doesn't have any function like that so I wrote the following:

SELECT Unit, [Date], Mileage
FROM (
    SELECT a.id, a.Unit, a.[Date], a.Mileage
    FROM tbl_Unit a
        INNER JOIN (SELECT MAX(id) MaxID, Unit FROM tbl_Unit
                    GROUP BY Unit) b ON a.id = b.maxid
) t1

I'm looking for alternates or better code.
This query returns one record for each "Unit" where the Record is the LAST record entered regardless of the date or mileage values entered. The Table has several thousand entries and this query returns over 100 rows.

The ID is both an auto incrementing number and the primary key. The following data is part of the table.

ID      Unit    DATE        Mileage
217316  171     2006-01-27  59761
216668  171     2005-12-01  57875
216194  171     2006-01-21  59346
217591  1127    2006-01-30  406692
217467  1127    2006-01-27  406339
217466  1127    2006-01-27  406127
217598  2310    2006-01-29  68372
217505  2310    2006-01-28  68187
217504  2310    2006-01-28  67987

The correct output with this set of data is:

Unit    Date        Mileage
171     2006-01-27  59761
1127    2006-01-30  406692
2310    2006-01-29  68372

This should do it:

SELECT U1.Unit, U1.[Date], U1.Mileage
FROM tbl_unit U1
WHERE U1.id IN (
    SELECT MAX(U2.id)
    FROM tbl_unit U2
    GROUP BY U2.Unit
);

This uses a simple subquery correlated on the tbl_Unit.Unit.  This approximates your GROUP BY and LAST() functions in Access by SELECTing the most recent row for each Unit.  LAST() in Access returns the most recently added record, and since the id field is an Autonumber (or IDENTITY() in SQL Server) field, MAX(id) will automatically return the most recent row for each unit.


Finding values below a threshold I am wondering if it's possible to avoid looping. I want to make the function faster. The function is to find how many times a particular value is lower in the overall data. For example:

Value <- c(0,10,5,1,0,0,11,0,0,7,3,2,5) # length =13

The 1st value is 0 and 0 is NOT lower in any of the values. Therefore the value I would like to return from the function would be 0.

Something like this:

sum(Values < 0)
0

The 2nd value is 10:

sum(Values < 10)
11

I make a function something like below:

fun_num <- function(Value){
  gg <- vector()
  for (i in seq_along(Value)){
    x <- Value[i]
    y <- sum(Value < x)
    gg[i] <- y
  }
  return(gg)
}

Applying the function:

fun_num(Value)

[1]  0 11  8  5  0  0 12  0  0 10  7  6  8

Just wondering, is there a way to speed this function up using sapply and avoid using for loop?

This should suffice

fun_2 <- function(x) sapply(x, function(y) sum(x < y))
fun_2(Value)
# [1]  0 11  8  5  0  0 12  0  0 10  7  6  8

P.S. sapply/lapply are the same loops, just masked and a little bit faster.

Or if your data is very large we can do it a lot faster using data.table, aggregating the data and counting:

require(data.table)
fun_3 <- function(Value) {
  d <- data.table(x = Value) # creates 1 column data.table
  # setkey(d, x) # not needed here keyby sets the key
  d <- d[, .N, keyby = x] # calculate count of each unique x value
  # and sorts the results
  d[, a := c(0, cumsum(N)[-.N])] # calculate lagged cumsum from N (counts)
  # a represents element count that is smaller than x
  d[.(Value), a] # using datatable-keys-fast-subset get a(result) for each Value
}

set.seed(42)
Value2 <- sample.int(1e5)

system.time(r2 <- fun_2(Value2)) # 36.64
system.time(r3 <- fun_3(Value2)) # 0.03
all.equal(r2, r3)
# [1] TRUE

OR with base R:

fun_4 <- function(x) {
  xorder <- order(x)
  xsorted <- x[xorder]
  xsdifs <- c(0, diff(xsorted))
  m <- seq_along(xsdifs) - 1L
  m[xsdifs == 0L] <- 0L
  m <- cummax(m)
  m[order(xorder)]
}

For reverse (Value > x):

fun_2r <- function(x) sapply(x, function(y) sum(x > y))
fun_3r <- function(Value) {
  d <- data.table(x = Value)
  d <- d[, .N, keyby = x]
  setorder(d, -x)
  d[, a := c(0, cumsum(N)[-.N])]
  setkey(d, x) # need to reset key for sub setting,
  # because reordering d removes it
  d[.(Value), a]
}


Comparing 2 CSV files Here's the exercise in brief:

  Consider the following file: Code:

  before.csv

A; ; B;
B; A; H;
C; ; D;
D; C; G;
E; D; F;
F; E; H;
G; D; ;
H; G; ;

  And a modified version of the file:

  after.csv

A; ; B;
B; A; H;
C; ; D;
D; ; G;
E; D; F;
F; E; H;
G; D; ;
K; ; E;

  The first field of the CSV is a unique identifier of each line. The
  exercise consists of detecting the changes applied to the file, by
  comparing before and after.

  There are 3 types of changes you should detect:

  ADDED (line is present in after.csv but not in before.csv)
  REMOVED (line is present in before.csv but not in after.csv)
  MODIFIED (line is present in both, but second and/or third field are modified)

  In my example, there are three modifications:

  ADDED line (K)
  REMOVED line (H)
  MODIFIED line (D)

And my code:

import collections
import csv
import sys

class P_CSV(dict):
    '''A P_CSV is a dict representation of the csv file:
       {"id": dict(csvfile)} '''

    fieldnames = ["id", "col2", "col3"]

    def __init__(self, input):
        map(self.readline, csv.DictReader(input, self.fieldnames, delimiter=";",\
                                              skipinitialspace=True))

    def readline(self, line):
        self[line["id"]] = line

    def get_elem(self, name):
        for i in self:
            if i == name:
                return self[i]

class Change:
    ''' a Change element will be instanciated
        each time a difference is found'''.

    def __init__(self, *args):
        self.args=args

    def echo(self):
        print "\t".join(self.args)

class P_Comparator(collections.Counter):
    '''This class holds 2 P_CSV objects and counts
       the number of occurrence of each line.'''

    def __init__(self, in_pcsv, out_pcsv):
        self.change_list = []

        self.in_pcsv = in_pcsv
        self.out_pcsv = out_pcsv

        self.readfile(in_pcsv, 1)
        self.readfile(out_pcsv, -1)

    def readfile(self, file, factor):
        for key in file:
            self[key] += factor

    def scan(self):
        for i in self:
            if self[i] == -1:
                self.change_list.append(Change("ADD", i))
            elif self[i] == 1:
                self.change_list.append(Change("DELETE", i))
            else: # element exists in two files. Check if modified
                j = J_Comparator(self.in_pcsv.get_elem(i), self.out_pcsv.get_elem(i))
                if len(j) > 0:
                    self.change_list += j

class J_Comparator(list):
    '''This class compares the attributes of two lines and return a list
       of Changes object for every difference'''

    def __init__(self, indict, outdict):
        for i in indict:
            if indict[i] != outdict[i]:
                self.append(Change("MODIFY", indict["id"], i, indict[i], "BECOMES", outdict[i]))
        if len(self) == 0:
            self = None

#Main
p = P_Comparator(P_CSV(open(sys.argv[1], "rb")), P_CSV(open(sys.argv[2], "rb")))
p.scan()

print "{} changes".format(len(p.change_list))
[c.echo() for c in p.change_list]

In real life, the code is supposed to compare two very large files (>6500 lines) with much more fields (>10).

How can I improve, both the style of my programming as well as the performance of the script? For the record I'm using Python2.7

Implementation

The P_CSV trick is good idea.
I don't know if "input" is supposed to be a file name, a string, a file object, and so on (this is Python's fault, but still). Please use a better name and document that it is a file.
What does {"id": dict(csvfile)} mean in your docstring?
get_elem could be implemented with return self.get(name, default=None). The way you're doing it is misleading, since you're relying on the fact that no return means return None.
This means you can either remove get_elem or find a better name explaining that it's just like get except that it returns None instead of throwing an exception.
I guess you want to use __str__ in Change, and maybe __repr__ (but not echo).
Do you really need Change as it is? Simply store lists in change_list, instead of Changes.
readfile? I'd say readcsv since it is no longer a file, but a P_CSV. If you have a more descriptive name of what those files contain, then use that instead.
J_Comparator doesn't work as requested in the exercise, since it also says what columns were modified.
Setting a list to None is wrong. "No values" is the empty list. You can then use self.change_list.extend(j) without needing to worry about the empty list. It's much more elegant.
Why P and J for the comparators?

Performance

Performance is good: you're using a linear algorithm, even though you're going through the files twice. If you're worried about very very larges files that won't hold in memory, you can use the assumption that the files are sorted to advance in both files simultaneously, and make sure to always have the same unique id in both files. I don't think this is needed, 6000 lines is quite small!


Excel VBA highlighting macro I have this Excel macro I created to highlight all instances of a number if at least one instance is already highlighted before running the macro.

Sub highlightXIDs()
    Dim prods As Object: Set prods = CreateObject("Scripting.Dictionary")
    Dim lastRow As Long: lastRow = ActiveSheet.Range("A" & Rows.Count).End(xlUp).Row
    Dim tRange As Range

    For Each tRange In ActiveSheet.Range("A2:A" & lastRow)
        If tRange.Interior.ColorIndex <> xlNone Then prods.Add Key:=tRange.Value, Item:=tRange.Interior.Color
    Next

    Dim prod As Variant, xidMap As Object
    Set xidMap = getXidMap(ActiveSheet.Range("A2:A" & lastRow))
    For Each prod In prods.keys
        xidMap(prod).EntireRow.Columns("A").Interior.Color = prods.Item(prod)
    Next prod
End Sub

'get a "map" of each unique xid value to the rows containing it
Function getXidMap(rng As Range) As Object
    Dim rv As Object, c As Range, currVal, cStart, i, tmp
    Set rv = CreateObject("scripting.dictionary")
    For Each c In rng.Cells
        tmp = c.Value
        If Len(tmp) > 0 Then
            If rv.exists(tmp) Then
                Set rv(tmp) = Application.Union(c, rv(tmp))
            Else
                rv.Add tmp, c
            End If
        End If
    Next c
    Set getXidMap = rv
End Function

Before:

After:

Is this an efficient use of the included vba objects or should I change something?
For the line where I check the cell color, which would be more accurate/efficient in finding any cell that has fill color (excluding conditional formatting):

.Interior.ColorIndex <> xlNone

Or

.Interior.Color <> -4142

Or would these both work the same with the same amount of accuracy?

I'm going to talk about variable naming. I can't read your code and understand what's happening which indicates that your code isn't self-explanatory. One step in accomplishing that is to give variables meaningful names:

lastRow is good! Otherwise..

prods - what is this? a dictionary of product keys? why not productList or something similar?

prod how is this different than prods?  Should it be productListKey?

tRange, rv, c, i and tmp - I have no idea what they should be doing - except for i because it's pretty standard.

cStart, currVal and i - you never use them. But if you did  - why not just use the entire word for the description? currentValue and cellStart

Speaking of currVal, cStart, i and tmp - When you don't define your variable, VBA will declare it as a Variant, which are objects:

  Performance. A variable you declare with the Object type is flexible
  enough to contain a reference to any object. However, when you invoke
  a method or property on such a variable, you always incur late binding
  (at run time). To force early binding (at compile time) and better
  performance, declare the variable with a specific class name, or cast
  it to the specific data type.

By not declaring variables, you could possibly be paying a penalty.

What is rv anyway?

Also you are using (rng as Range) in your function - but you're passing it ByRef by default. I don't see any need to do that, so ByVal testRange as Range would be better.

Logic

As far as I can tell you make a dictionary of all values with an interior color. Then you make a dictionary of all values `in the same range as the first dictionary. Then you compare the dictionaries. Since you're already looping through the range for the second dictionary, I'm not sure what kind of speed gains you're getting considering you could just loop through once.

As for finding unfilled cells - I think they are the same. The color is more accurate than colorindex. I'd still use the color property though as all the rest of my code would use color before colorindex.


Convert three functions into a single function Is it possible to combine all the functions into a single function?  I'm providing my fiddle as well.

I am trying to test it for different scenarios..
but I am not sure how to combine all the scenarios into one..

var myNumbersToSort = [null, -1, 2, 0.001, -3, 4, 0.3,1,-0.0001];

function getClosestToZero(set) {
  if(0 === set.length) return null;
  var closest = set[0], result = 0;
  for(var i in set) {
     var next = set[i];
     if(Math.abs(closest) > Math.abs(next)) {
       result = i;
       closest = next;
     }
  }
  return closest;
}

function getClosestToZeroWithNaN(set) {
    var closest;
    if(set instanceof Array) {
        for(var i = 0; i < set.length; i++) {
            var val = set[i];
            if(!isNaN(val)){
                val = Number(val);
                var absVal = Math.abs(val);
                    if(typeof closest == 'undefined' || Math.abs(closest) > absVal)                       {
                    closest = val;
                }
            }
        }
    }
    return closest;
}
function getClosestToZeroOnlyNumbers(set) {
    var closest;
    if(set instanceof Array) {
        for(var i = 0; i < set.length; i++) {
            var val = set[i];
            if(typeof val == "number"){
                var absVal = Math.abs(val);
                    if(typeof closest == 'undefined' || Math.abs(closest) > absVal)                       {
                    closest = val;
                }
            }
        }
    }
    return closest;
}

document.getElementById('output').innerHTML = (getClosestToZeroOnlyNumbers(myNumbersToSort));

I think your code would be cleanest if you wrote the program as a composition of a minimum-absolute-value-of-array function and a filter.

Assuming that performance in the face of large data sets is not a concern, you could have, for example:

function isNumber(val) {
    return typeof val == "number";
}

getClosestToZero(myNumbersToSort.filter(isNumber));

Alternatively, getClosestToZero() could directly support an optional second argument that servers as a filtering callback if it is provided.

Please pay attention to the consistency of the indentation.


"Changing Bits" challenge Problem Statement:
  Let A and B be two N bit numbers (MSB to the left). You are given initial values for A and B, and you have to write a program which processes three kinds of queries:

  set_a idx x: Set A[idx] to x, where \$0 \le idx < N\$, where A[idx] is idx'th least significant bit of A.
  set_b idx x: Set B[idx] to x, where \$0 \le idx < N\$, where B[idx] is idx'th least significant bit of B.
  get_c idx: Print C[idx], where \$C=A+B\$, and \$0\le idx\$

  Input Format:
  First line of input contains two integers \$N\$ and \$Q\$ consecutively (\$1 \le N \le 100000\$, \$1\le Q \le 500000\$). Second line is an \$N\$-bit binary number which denotes initial value of \$A\$, and the third line is an \$N\$-bit binary number denoting initial value of \$B\$. \$Q\$ lines follow, each containing a query as described above.

  Output Format:
  For each query of the type get_c, output a single digit \$0\$ or \$1\$. Output must be placed in a single line.

  Sample Input:

5 5
00000
11111
set_a 0 1
get_c 5
get_c 1
set_b 2 0
get_c 5

  Sample Output:

100

I solved this question and I tried to optimize by reducing the number of calculations for bit addition, but am still getting time-limit-exceeded where the time exceeds 10 seconds for Python.

n, q = map(int, raw_input().split())
A_inp = list(raw_input())
B_inp = list(raw_input())
Query_output = []

def LSB(idx, n):
    return (n - 1) - (int(idx))

def Bit_Add(a, b, n):
    length = len(a)
    output = []
    carry = 0
    for i in range(length):
        if a[i] == "1" and b[i] == "1":
            if carry == 1:
                output.append(1)
                carry = 0
            else:
                output.append(0)
                carry = 1

        elif a[i] == "1" or b[i] == "1":
            if carry == 1:
                output.append(1)
                carry = 0
            else:
                output.append(0)
        else:
            if carry == 1:
                output.append(1)
                carry = 0

    if carry == 1:
        output.append(1)
    return output[::-1]
f = 0
for i in range(q):
    query = raw_input().split()
    if query[0][0:3] == "set":
        Value = query[0][4]
        idx = LSB(int(query[1]), n)
        if Value == "a":
            A_inp[idx] = query[2]
        elif Value == "b":
            B_inp[idx] = query[2]
        f = 0
    else:
        if f == 1:
            idx = LSB(query[1], len(output))
            Query_output.append(output[idx])
        else:
            output = Bit_Add(A_inp, B_inp, n)
            idx = LSB(query[1], len(output))
            Query_output.append(output[idx])
            f = 1

print int(''.join(["%d"%x for x in Query_output]))

The trick to this challenge is to actually change the number into bit operations, and not to do string manipulation as you do. So here are some hints on how to do this to get you started:

Convert a binary string, text, of 0's and 1's into an int, use int(text, 2)
To clear a bit at a given offset, use n & ~(1 << offset)
To set a bit at a given offset, use n | (1 << offset)
To get a bit at a given offset (shifted down again to one bit), use (n & (1 << offset)) >> offset)

Combine these, and you'll get a fast enough solution.

Style review of current code

Some tips according to PEP8:

Use lower_snake_case for variable and function names -  This looks a little better, and wouldn't throw off the syntax highlighter here on Code Review either... :-)
Add a little vertical space in your code – Add two blank lines between functions, and add a blank line in natural segments within functions, like before if, else, for and while.
Put all top level code in a main() function – Try to only keep imports, constants and functions at the top level, and start your code using a if __name__ == '__main__':' followed bymain()`
Start using the print function, and str.format'ing– The % syntax will disappear some time, so start using the newer string formatting options already available.
For Python 2 and large \$n\$'s use xrange() in for loops – Using range() will create the entire list, whilst xrange() will create a generator, and only give you one element at a time. For large numbers this has a noticeable effect on memory and time performance
No need to create a list for join() – Instead of ''.join([ ... ]), you can simply use ''.join( ... ). The latter uses a generator instead of a list, similar to the range() vs xrange()

Refactored code running within time limit

So as not to spoil the fun for those wanting to solve this challenge by themselves, I've chosen to have the code hidden beneath a spoiler. That is if you hover over the block below you'll see the code.

  def getBit(n, offset):
    return (n & (1 << offset)) >> offset

def clearBit(n, offset):
    return n & ~(1 << offset)

def setBit(n, offset):
    return n | (1 << offset)

def changing_bits_challenge():

    n, q = map(int, raw_input().split())
    a = int(raw_input(), 2)
    b = int(raw_input(), 2)

    c = []
    for _ in xrange(q):
        words = raw_input().split()
        operator = words[0][4]
        bit_number = int(words[1])

        if operator == 'c':
            c.append(getBit(a+b, bit_number))

        if operator == 'a':
            a = clearBit(a, bit_number) if words[2] == '0'
                else setBit(a, bit_number)

        if operator == 'b':
            b = clearBit(b, bit_number) if words[2] == '0'
                else setBit(b, bit_number)

    print ''.join(str(i) for i in c)

changing_bits_challenge()


Slick Slider – change contents based on current slide data-index I wrote the following code for Slick Slider, which changes the content below based on the current slide data-index. I just wonder if there is a more efficient way to do it?

http://codepen.io/pjmtokyo/pen/JYyjew

$('.slider').on('afterChange', function(event, slick, currentSlide, nextSlide){
    var elSlide = $(slick.$slides[currentSlide]);
    var dataIndex = elSlide.data('index');

  $('.content').each(function(){
    if ($(this).data('id') == dataIndex) {
      $(this).show();
    } else {
      $(this).hide();
    }
  });

});

I would combine the two lines where you get the dataIndex, rename that to dataId to be consistent with the HTML element, and replace the loop by hiding everything, and then showing the correct element by finding it with a jQuery selector:

$('.slider').on('afterChange', function(event, slick, currentSlide, nextSlide){
    var dataId = $(slick.$slides[currentSlide]).data('index');
    $('.content').hide();
    $('.content[data-id=' + dataId + ']').show();
});

I also noticed that looking up dataId is superfluous, since it is simply the value of currentSlide plus one.  So you could also make this change and retain the same behavior:

$('.slider').on('afterChange', function(event, slick, currentSlide, nextSlide){
  $('.content').hide();
  $('.content[data-id=' + (currentSlide + 1) + ']').show();
});