Find lowest consecutive value I have created a program where I first set the size of the array and
how many times I will look for consecutive values, then I fill in the
array and finally I insert the consecutive values, it should look for
the least consecutive value it finds in the array.
consecutive value means the sum of the elements in the array
consecutively.
example:
7 4 // Array size and consecutive values
6 4 2 7 10 5 1 // arrangement
1 3 4 2 // Consecutive values

Answer = 1,12,19,6

Explanation

The lowest element of the array is the one in the last position which
is 1, the answer is equal to 1.
the lowest segment is the one furthest to the left 6 4 2, the sum of
which equals 12.
the answer is obtained by choosing segment 6 4 2 7, which equals 19.
there are two segments with a minimum cost equal to 6, segments 4 2
and 5 1.

How can it be improved?
#include<iostream>
#include<vector>
using namespace std;

void suma_elementos(int elemento);
vector<int>elementos, consecutivos;
int n, q, consecutivo, total, final = 99999999;

int main()
{
    cin >> n >> q;
    elementos.resize(n);
    consecutivos.resize(q);

    for (int i = 0; i < n; i++)
    {
        cin >> elementos[i];
    }
    for (int i = 0; i < q; i++)
    {
        cin >> consecutivos[i];
    }

    for (int i = 0; i < q; i++)
    {
        suma_elementos(consecutivos[i]);

        for (int j = 0; j < consecutivo; j++)
        {
            for (int c = 0; c < consecutivos[i]; c++)
            {
                total += elementos[c + j];
            }

            if (total < final)
            {
                final = total;
            }
            total = 0;
        }
        cout << final << " ";
        //reset
        consecutivo = 0;
        final = 99999999;
        total = 0;
    }

    return 0;
}

//Suma de elementos
void suma_elementos(int elemento)
{

    int proporcion = n;

    while (proporcion >= elemento)
    {
        proporcion--;
        consecutivo++;
    }
}

edit 1:The program in small arrangements works well, but in large ones it's very slow
edit 2:must not have negative numbers in the sequence

Avoid globals. consecutivo is particularly confusing. It is too easy to miss the fact that it is reset to 0 at each iteration. Always prefer returning a value:
int suma_elementos(int elemento)
{
    int proporcion = n;
    int consecutivo = 0;
    while (proporcion >= elemento)
    {
        proporcion--;
        consecutivo++;
    }
    return consecutivo;
}

and use it in your loop as
    int consecutivo = suma_elementos(consecutivos[i]);

Now it is obvious that suma_elementos does not really need to loop. You have an invariant proporcion + consecutivo == n. At the end of the last iteration you also have proporcion == elemento, which means that consecutivo = n - elemento. In other words,
int suma_elementos(int elemento)
{
    return n - elemento;
}

Now I would rather not bother to have suma_elementos at all (besides, with my poor Spanish I have an impression that the name is rather misleading). The loop would become
    for (int j = 0; j < n - consecutivos[i]; j++)

Honestly, it is much easier to understand.

final = 99999999; is very fragile. Consider a ramp-up loop:
  final = 0;
  for (int j = 0; j < consecutivos[i]; j++) {
      total += elementos[j];
  }

Your code complexity is \$O(n*k)\$ (k being a width of the window). Notice that after the ramp-up you don't need to recompute the sums of k elements: as the window shifts, one element enters, another leaves:
  final = total;
  for (int leave = 0, enter = base[i]; enter < n; leave++, enter++) {
      total += elementos[enter] - elementos[leave];
      if (total < final) {
          final = total;
      }
  }

obtaining \$O(n + k)\$ complexity.


Thread-safe callback registration and invocation in C++11 I need your help in reviewing the following code. My main concern is dead-lock, but there might also be other issues.

The EventGenerator object is owned by a single thread that running the ThreadTask.  Other threads might access EventGenerator public APIs to set/remove event handlers.

A few corner cases that I've thought about:

The thread that is executing ThreadTask might call EventGenerator::{Add,Remove}Handler via the EventDelegate::Invoke.
The registered EventDelegate might be cancelled at any time. If EventDelegate::Cancel was called before the invocation, the invoke should not happen, if was called during/after, then nothing should happen.

#include <thread>
#include <atomic>
#include <mutex>
#include <iostream>
#include <vector>
#include <memory>
#include <chrono>

class IEventHandler {
public: virtual void onEvent(int data) = 0;
};

class EventDelegate {
private:
    IEventHandler* _handler;
    std::atomic<bool> _cancelled;
 public:
     EventDelegate(IEventHandler* h) : _handler(h), _cancelled(false) {};
     void Cancel() { _cancelled = true; }
     void Invoke(int data) { if (!_cancelled) _handler->onEvent(data); }
 };

 class EventGenerator {
 private:
     std::vector<std::shared_ptr<EventDelegate>> _handlers;
     std::mutex _mutex;
 public:
     void AddHandler(std::shared_ptr<EventDelegate> handler) {
         std::lock_guard<std::mutex> lck(_mutex);
         _handlers.push_back(handler);
     }
     void RemoveHanler(std::shared_ptr<EventDelegate> handler) {
         std::lock_guard<std::mutex> lck(_mutex);

         for (auto it = _handlers.begin(); it != _handlers.end(); ++it) {
             if ((*it) == handler) { _handlers.erase(it); break; }
         }
     }
     void ThreadTask() {
         int data = 0;

         while (true) {

             std::this_thread::sleep_for(std::chrono::seconds(1));
             data++;

             std::vector<std::shared_ptr<EventDelegate>> handlers_copy;

             {
                 std::lock_guard<std::mutex> lck(_mutex);
                 handlers_copy = _handlers;
             }

             for (auto& h : handlers_copy) { h->Invoke(data); }

             if (data == 15) { break; }

         }
    }
};

// Concrete class
class MyEventHandler : public IEventHandler {
public:
    virtual void onEvent(int data) {
        std::cout << "onEvent: data = " << data << std::endl;
    }
};

int main(int argc, char** argv) {
    EventGenerator generator;
    std::thread t(&EventGenerator::ThreadTask, &generator);

    MyEventHandler h;
    std::shared_ptr<EventDelegate> deleg = std::make_shared<EventDelegate>(&h);

    generator.AddHandler(deleg);
    std::this_thread::sleep_for(std::chrono::seconds(3));

    generator.RemoveHanler(deleg);
    std::this_thread::sleep_for(std::chrono::seconds(3));

    generator.AddHandler(deleg);
    std::this_thread::sleep_for(std::chrono::seconds(3));

    deleg->Cancel();
    std::this_thread::sleep_for(std::chrono::seconds(2));

    t.join();
    return 0;
}

From your concerns:

EventGenerator::{Add,Remove}Handler should be safe to call from EventDelegate::Invoke, though any updates won't be reflected in the current loop iteration in ThreadTask.
Your shared_ptrs are pointing to the same EventDelegate class, and you're using an atomic bool. This should behave properly.

My concerns are more minor. Your test code is too simple to catch many bugs (if they exist). Your ThreadTask function should use a for loop rather than while(true). You should consider using the remove/erase idiom in RemoveHanler. Finally, your code allows multiple copies of the same handler to be added. I don't know if that's intentional or not.


Filter for logging http request parameters I have written a filter that intercepts http calls and logs request parameters.

public void doFilter(ServletRequest request, ServletResponse response,
        FilterChain filterChain) throws IOException,
        ServletException
    {
        HttpServletRequest httpRequest = (HttpServletRequest) request;
        HttpServletResponse httpResponse = (HttpServletResponse) response;
        String uri = httpRequest.getRequestURI();
        String queryString = httpRequest.getQueryString();
        long startTime = System.currentTimeMillis();
        if (shouldBeRedirected(uri)) {
            String type = getType(uri);
            String redirectUrl = Util.getURLBasedUponStabilityLevel(Constants.STABILITY_LEVEL);
            ((HttpServletResponse) response).sendRedirect(redirectUrl + type);
            return;
        }
        filterChain.doFilter(httpRequest, httpResponse);
        long endTime = System.currentTimeMillis();
        Timestamp startTimestamp = Timestamp.getInstance(startTime);

        StringBuilder logString = new StringBuilder();
        logString.append("Request URL: ");
        logString.append(uri);
        logString.append(" Query String: ");
        logString.append(queryString);
        logString.append(" Session Id: ");
        logString.append(httpRequest.getRequestedSessionId());
        logString.append(" Took: ");
        logString.append(endTime - startTime);
        logString.append("milliseconds. ");
        logString.append(" StartTime: ");
        logString.append(startTimestamp);
        if(httpRequest.getMethod().equals("POST")){
            JSONObject postParameters = new JSONObject();
            Map parameterMap = httpRequest.getParameterMap();
            Iterator<Map.Entry<String, String[]>> it = parameterMap.entrySet().iterator();
            while (it.hasNext()) {
               Map.Entry<String, String[]> entry = it.next();
               if(entry.getValue().length == 1){
                   postParameters.put(entry.getKey(), entry.getValue()[0].toString());}
               else{
                  JSONArray valueList = new JSONArray();
                  for(String value : entry.getValue()){
                      valueList.add(value.toString());
                  }
                  postParameters.put(entry.getKey(), valueList);
               }
            }
            logString.append(" Post Parameters: ");
            logString.append(postParameters.toString());
        }
        LOG.info(logString.toString());
    }

Code looks a bit clumsy to me, any suggestions to improve this?

The redirect doesn't seem to be part of the logging. Maybe put it in a separate filter that runs before this one?

The logged string could be built using String.format() or MessageFormat which would be more readable. Most logging frameworks have such formatting built into their logging methods anyway.

Do you really need to convert the post parameters into JSON? Isn't the standard toString representation of the Map good enough for logging? Apart from that most JSON libraries have a built-in mechanism to convert a Map into a JSON object so you don't need to do it yourself.


Drawing a checkerboard with FLTK This is my first attempt to draw using FLTK libraries *:

// Objective: Draw an 8-by-8, red and white checkers board

#include "std_lib_facilities.h"
#include <iostream>
#include "Simple_window.h"

int main(){
try{
    // create a window in the center of the screen wiht size: 600x600
    int wWidth = 660;
    int wHeight = 660;
    Point centerScreen(x_max()/2 - wWidth/2, y_max()/2 - wHeight/2);
    Simple_window sw(centerScreen, 660, 660, "Chapter 12 Exercise 3");

    // starting upper left coordinates of the window + 10 pixels frame
    int tlx = sw.x_max() - 650;
    int tly = sw.y_max() - 650;
    // Point topLeftCheckers(tlx, tly);

    // instantiate all the squares as rectangle objects with: heigth = width
    int numOfColumns = 8;
    int numOfRows = 8;

    // sqrSize == step of drawing
    int sqrSize = 80;

    // vector holding all (pointes to) rectangles
    vector<Graph_lib::Rectangle*> rects;
    int numOfSquares = numOfColumns *  numOfRows;
    rects.reserve(numOfSquares);

    for(size_t i=0; i < numOfColumns; ++i){
        for(size_t j=0; j < numOfRows; ++j){
            // create a 64 conjugate squares with size = 80,
            Graph_lib::Rectangle* r = new Graph_lib::Rectangle(Point(tlx + sqrSize*i, tly + sqrSize*j), sqrSize, sqrSize);
            // fill the consequtive squares with white or red color
            if ((i+j) % 2 == 0) r->set_fill_color(Color::red);
            else r->set_color(Color::white);
            // save object in vector
            rects.push_back(r);
        }
    }
    // attach all the rectangle objects to the window object
    for(size_t k=0; k < rects.size(); ++k) sw.attach(*rects[k]);

    sw.wait_for_button();

}catch(exception& e){
    cerr << e.what() << endl;
    getchar();

}catch(...){
    cerr <<"Default exception!"<< endl;
    getchar();
}
return 0;
}

Questions:

Are there any optimizations (or anything better) that can be done on the above code?
Any useful tips and practices that one should know in the early stages, when using GUI's packages like FLTK?

*as part of B. Stroustrup's "Principles and practice of C++".
All the needed files for compilation are here.
The FLTK could be found here

int wWidth = 660;
    int wHeigth = 660;

If you never change these, you may as well make them constants.

    const int wWidth = 660;
    const int wHeight = 660;

Then it's clear that they aren't intended to change.

I also fixed the spelling of "height".

    Simple_window sw(centerScreen, 660, 660, "Chapter 12 Exercise 3");

You just created variables for this.   Why not use them?

    Simple_window sw(centerScreen, wWidth, wHeight, "Chapter 12 Exercise 3");

Then you can change one value and have it propagate through.

    int tlx = sw.x_max() - 650;
    int tly = sw.y_max() - 650;

Same thing here.

    const int FRAME_SIZE = 10;
    const int TOP_LEFT_X = sw.x_max() - wWidth + FRAME_SIZE;
    const int TOP_LEFT_Y = sw.y_max() - wHeight + FRAME_SIZE;

This would also save you a comment, as it is much more obvious what this does.

        for(size_t j=0; j < numOfColumns; ++j){

It won't make a functional difference if both are 8, but this should be

        for (size_t j = 0; j < numOfRows; ++j) {

Otherwise if you change one value without changing the other (e.g. if you wanted a 12x8 checkerboard), it won't draw the board as you designate it.

            if ((i+j) % 2 == 0) r->set_fill_color(Color::red);
            else r->set_color(Color::white);

It's more robust in the face of future edits to write this as

            if ((i + j) % 2 == 0) {
                r->set_fill_color(Color::red);
            } else {
                r->set_color(Color::white);
            }

Also consider

            r->set_fill_color(((i + j) % 2 == 0) ? Color::red : Color::white);

Which is more direct.

    for(size_t k=0; k < rects.size(); ++k) sw.attach(*rects[k]);

Why do this separately?  Instead, put it in the j loop, at the end.

            sw.attach(*r);

Note that you can do this even if you put it in rects as well.

For this code, you don't need rects at all.  Perhaps you're planning on using it later, but you don't really need it now.

In C++, when you use the new operator, the memory remains allocated until you explicitly delete it.  You may want to use smart pointers.  Those would work more like you think, with garbage collection when there are no references left.

Also, if you hadn't used new but instead created a local variable that would have been automatically destroyed, putting it in rects wouldn't have worked.  The rects vector would have just contained a bunch of references to memory that no longer represented what it had when it was created.


"Magic Squares" assignment The assignment is to take a "Magic Square" .txt file (where the horizontal lines, vertical lines and diagonal line all add up to the same number), read it and prove that they are in fact magic squares.

Is anything in this answer bad practice or just bad? I'm using a lot of Lists and for loops.

package magicSquares;

public  MagicSquaresCormac() {
    return;
}

private List<List<Integer>> getNumberList(String file) throws IOException {
    FileReader fr = new FileReader(file);
    BufferedReader br = new BufferedReader(fr);

    String line = null;
    Integer num = null;
    List<String> lineList = new ArrayList<>();
    List<String> charList;
    List<List<String>> listOfNums = new ArrayList<>();
    List<Integer> numlist;
    List<List<Integer>> numListList = new ArrayList<>();

    while ((line = br.readLine()) != null) {
        line.split("\t");
        if (!line.equals("")) {
            lineList.add(line);
        }
    }

    for (String lineFromList : lineList) {
        //    lineFromList.split(" ");
        numlist = new ArrayList<>();
        charList = Arrays.asList(lineFromList.split("\\s"));
        listOfNums.add(charList);
        for (String character : charList) {
            num = Integer.valueOf(character);
            numlist.add(num);
        }
        numListList.add(numlist);
    }

    br.close();
    return numListList;
}

private boolean horizontalLine(String file) throws IOException {
    List<List<Integer>> numListList = getNumberList(file);
    boolean isMagic = true;
    Integer lastSum = -1;
    Integer sum;

    for (List<Integer> listNum : numListList) {
        sum = 0;
        for (Integer i : listNum) {
            sum = sum + i;
        }
        if (lastSum == -1)
            lastSum = sum;

        else if (!lastSum.equals(sum)) {
            isMagic = false;
        }
    }

    return isMagic;
}

private boolean diagonalLine(String file) throws IOException {
    List<List<Integer>> numListList = getNumberList(file);
    boolean isMagic = true;
    Integer sum;
    Integer sumDiagonal = 0;

    for (int j = 0; j < numListList.size(); j++) {
        sumDiagonal = sumDiagonal + numListList.get(j).get(j);
    }

    sum = sumDiagonal;

    if (!sum.equals(260)) {
        isMagic = false;
    }
    return isMagic;
}

private boolean verticalLine(String file) throws IOException {
    List<List<Integer>> numListList = getNumberList(file);
    boolean isMagic = true;
    Integer lastSum = -1;
    Integer sum;
    List<Integer> tempList;
    List<List<Integer>> verticalLists = new ArrayList<>();

    for (int k = 0; k < numListList.size(); k++) {
        tempList = new ArrayList<>();
        for (int i = 0; i < numListList.size(); i++) {
            tempList.add(numListList.get(i).get(k));
        }
        verticalLists.add(tempList);
    }

    for (List<Integer> listNum : verticalLists) {
        sum = 0;
        for (Integer i : listNum) {
            sum = sum + i;
        }
        if (lastSum == -1)
            lastSum = sum;

        else if (!lastSum.equals(sum)) {
           System.out.println((lastSum != sum) + "really?");
            isMagic = false;
        }
    }
    return isMagic;
}

public static void main(String[] args) throws IOException {
   MagicSquaresCormac magicSquaresCormac =  new MagicSquaresCormac();
    String[] files = {"Mercury.txt", "Luna.txt"};
    for (String file : files) {
        System.out.println(file + " (horizontal lines) is magic? " + magicSquaresCormac.horizontalLine(file));
        System.out.println(file + " (vertical lines) is magic? " + magicSquaresCormac.verticalLine(file));
        System.out.println(file + " (diagonal line) is magic? " + magicSquaresCormac.horizontalLine(file));
    }
}
}

I suspect that you should read the matrices from files only once and store them in a two-dimensional integer array. Once you have that, it is trivial to verify whether the squares are magic.

I had in mind something like this:

private static int[][] loadFile(File file) {
     // Do file I/O for loading the square
     return square;
}

private static int getRowMagic(int[][] square, int row) {
    int sum = 0;

    for (int column = 0; column < square.length; ++column) {
        sum += square[row][column];
    }

    return sum;
}

private static int getColumnMagic(int[][] square, int column) {
    int sum = 0;

    for (int row = 0; row < square.length; ++row) {
        sum += square[row][column];
    }

    return sum;
}

private static int getDescendingDiagonalMagic(int[][] square) {
    int sum = 0;

    for (int i = 0; i < square.length; ++i) {
        sum += square[i][i];
    }

    return sum;
}

private static getAscendingDiagonalIsMagic(int[][] square) {
    int sum = 0;

    for (int i = 0; i < square.length; ++i) {
        sum += square[square.length - 1 - i][i];
    }

    return sum;
}

public static boolean isMagicSquare(int[][] square) {
    int sum = getRowMagic(square, 0);

    for (int i = 0; i < square.length; ++i) {
        if (sum != getRowMagic(square, i) || sum != getColumnMagic(square, i)) {
            return false;
        }
    }

    return getAscendingDiagonalMagic(square) == sum &&
           getDescendingDiagonalMagic(square) == sum;
}

Hope that helps.


Creating a generic filter system to pass into an API request I am building an application that will interact with an API.
My service layer will call the repository which will request data from the API.
I want to be able to pass filters to the repository and came up with the following. I am just curious if there are better ways of doing this?

First, I created an interface called IFilter:

public interface IFilter
{
    string GetFilterName();
    string GetFilterValue();
}

I then have a bunch of filters, for example:

public class CreatedAfterFilter : IFilter
{
    readonly string _name = "CreatedAfter";
    DateTime _value;

    public CreatedAfterFilter(DateTime value)
    {
        this._value = value;
    }

    public string GetFilterValue()
    {
        return _value.ToString("yyyy-MM-ddTHH:mm:ss+00:00");
    }

    public string GetFilterName()
    {
        return _name; ;
    }
}

...or

public class OrdersFilter : IFilter
{
    readonly string _name = "OrderIdList";
    List<Order> _value;

    public OrdersFilter(List<Order> value)
    {
        this._value = value;
    }

    public string GetFilterValue()
    {
        return '[' + String.Join(",", _value.Select(x => x.OrderId)) + ']';
    }

    public string GetFilterName()
    {
        return _name; ;
    }
}

When I call my repository, I do something like this:

var filters = new List<IFilter>();
filters.Add(new CreatedAfterFilter(DateTime.UtcNow.AddDays(-110)));
filters.Add(new OrderFilter(1709359));
var orders = orderRepository.GetSingleOrder(filters);

Then in my code that calls the API, I do this to create the query string:

foreach (var filter in filters)
{
   getParams.Add(new KeyValuePair<string, string>(filter.GetFilterName(), filter.GetFilterValue()));
}

Am I on the right track here?
This is my first real time in writing C# an using Interfaces.

Am I on the right track here?

I think you are. Here's how you can use a few tricks to make it even shorter.

Instead of the interface you can implement what is common in an abstract class.

public abstract class Filter
{
    public virtual string Name => Regex.Replace(GetType().Name, $"{nameof(Filter)}$", string.Empty);
    public static implicit operator string(Filter filter) => filter.ToString();
}

This means the base class will get the name of the filter from its type and it'll be able to implicitly cast it into a string by calling ToString. Now everything the derived class needs to do is to implement its own ToString:

public class CreatedAfterFilter : Filter
{
    private DateTime _value;

    public CreatedAfterFilter(DateTime value)
    {
        _value = value;
    }

    public override string ToString()
    {
        return _value.ToString("yyyy-MM-ddTHH:mm:ss+00:00");
    }
}

So later in the loop you can just do this:

foreach (var filter in filters)
{
   getParams.Add(new KeyValuePair<string, string>(filter.Name, filter));
}

If you make the Name property virtual you can override it in a derived class if the name of the filter cannot be derived from its type name:

public class OrdersFilter : Filter
{
    public override string Name => "OrderIdList";

    public override string ToString()
    {
        ...
    }
}


Load text file into an array in Swift I'm drowning in Swift optionals and error handling syntax. I'm just converting a text file into an array of Strings. One would think that this could be a simple two or three liner but by the time I got it to work, I ended up with the following mess:

enum ImportError: ErrorType {
    case FileNotFound
    case CouldntGetArray
}

private func getLineArrayFromFile(fileName: String) throws -> Array<String> {

    let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)
    if path == nil {
        throw ImportError.FileNotFound
    }

    var lineArray: Array<String>?
    do {

        let content = try String(contentsOfFile: path!, encoding: NSUTF8StringEncoding)
        lineArray = content.componentsSeparatedByString("\n")

    }catch{
        throw ImportError.CouldntGetArray
    }

    return lineArray!
}

I actually don't really care about the using ErrorType enum, but I wanted to play around with the new Swift Error Handling syntax.

I thought I understood optionals before, but they were giving me a headache when combined with the do-try-catch statement. I also didn't know if I should return an Array or an Optional Array.

What are the best practices for a situation like this?

Error handling
Treatment of optionals
Code brevity/readability

func getLineArrayFromFile(fileName: String) throws -> Array<String>

The function does not get lines from an arbitrary file, but from a resource file.
The "get" prefix is usually not used in Objective-C or Swift.
Array<String> can be shortened to [String].

So my suggestion for the function name would be

func linesFromResource(fileName: String) throws -> [String]

let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)
if path == nil {
    throw ImportError.FileNotFound
}

As you are obviously using Swift 2, this can be simplified with the guard
statement. Note that path is no longer an optional.
There are pre-defined NSError codes which can be used here instead of
defining your own. This also gives better error descriptions for free.

Example:

guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
    throw NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError, userInfo: [ NSFilePathErrorKey : fileName ])
}

do {
    let content = try String(contentsOfFile: path!, encoding: NSUTF8StringEncoding)
    lineArray = content.componentsSeparatedByString("\n")
} catch {
    throw ImportError.CouldntGetArray
}

You are catching the error and throw your own error code in the failure case,
so the actual error information is lost.
Better just call the try String(..) and let an error propagate to the
caller of your function:

let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)

Again, this gives better error descriptions for free.

So the complete method would now look like this (no optionals anymore,
no forced unwrapping with !):

func linesFromResource(fileName: String) throws -> [String] {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        throw NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError, userInfo: [ NSFilePathErrorKey : fileName ])
    }
    let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
    return content.componentsSeparatedByString("\n")
}

And a typical usage would be

do {
    let lines = try linesFromResource("file.txt")
    print(lines)
} catch let error as NSError {
    print(error.localizedDescription)
} catch let error {
    print(error)
}

The reason for the final catch let error is that it is required that the catch statements are exhaustive.
Even if we know that the function throws only NSErrors, the compiler doesn't know
that. (There are exceptions but that is a different topic .)

Now to your question how an error should be handled in general, and I would
say: it depends. There are tree different scenarios:

Loading the strings can fail, and you want to present or log an error message
in that case. Then the above method of throwing and catching an error is appropriate.
Loading the strings can fail, but the particular reason is of no interest
to the caller. In that case I would change the function to return an optional.

Example:

func linesFromResource(fileName: String) -> [String]? {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        return nil
    }
    do {
        let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
        return content.componentsSeparatedByString("\n")
    } catch {
        return nil
    }
}

 // Usage:
if let lines = linesFromResource("file.txt") {
    print(lines)
}

Finally, if failing to load the strings is a programming error then the function should abort in
the error case, and return the (non-optional) strings otherwise.

As an example, if this function is only used to load strings from fixed compiled-in
resource files which are supposed to exist, then failing to load a file would
be a programming error and should be detected early:

func linesFromResource(fileName: String) -> [String] {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        fatalError("Resource file for \(fileName) not found.")
    }
    do {
        let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
        return content.componentsSeparatedByString("\n")
    } catch let error {
        fatalError("Could not load strings from \(path): \(error).")
    }
}

let lines = linesFromResource("file.txt")
print(lines)

fatalError() prints a message before stopping execution of the program, which can
be helpful to locate the programming error. Otherwise you could shorten the
"forced" version to

func linesFromResourceForced(fileName: String) -> [String] {

    let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)!
    let content = try! String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
    return content.componentsSeparatedByString("\n")
}

which is the "three liner" that you were looking for in the introduction to
your question.


Number guessing game in Racket I decided to learn a Lisp-ish language and I am  discovering a very weird and new syntax. The obligatory first program, that also uses all the fundamentals, is the number guessing game.

Did I define too few or too many constants?
Is my code properly spaced and indented?
Is there a cleaner way to exit the loop than error?
Is it too imperative? Is it even possible to write the number guessing game functionally?
Is it OK to define null and set!?

#lang racket

(define NUM 12)
(define TRIES 6)
(define CORRECT "Correct! Well guessed!")
(define TOO_LOW "... Too Low ...")
(define TOO_HIGH "... Too high ...")

(define (num_message player_num)
  (if (= player_num NUM)
      CORRECT
       (if (> player_num NUM)
          TOO_HIGH
          TOO_LOW
      )
    )
  )

(define (number-guessing-game)
  (define out null)
  (for ([i (range TRIES)])
    (set! out (num_message (read)))
    (print out)
    (when (equal? out CORRECT)
        (print "You won")
        (error "Exiting")
    )
  )
  (print "Too many attempts")
)

(number-guessing-game)

Winning shouldn't be considered an error.  If you want a way to exit from the for-loop, use #:break.  But for and set! aren't really idiomatic in Lisp-like languages.  Instead, it is more common to use recursion and let, which conform to the functional programming paradigm.

In num_message, you can use a cond instead of nested ifs.

I'm not happy about the use of a string comparison to detect the winning condition: the number-guessing-game function needs to know what congratulatory message to expect from num_message.  You've worked around that problem by defining some string constants, which gives them more importance than one would expect.

It would be more elegant for num_message not to have NUM fixed, and for number-guessing-game not to have NUM and TRIES fixed.

The number-guessing-game function would be clearer if it were broken up, because it is actually doing two things: play one guess, and conditionally repeat.

The print calls should probably be displayln instead, to also write a trailing newline.

Conventionally, words in identifiers are separated-by-hyphens rather than underscores.  Closing parentheses don't get their own line.

#lang racket

(define NUM 12)
(define TRIES 6)

(define (num-message delta)
  (cond [(< delta 0) "... Too low ..."]
        [(> delta 0) "... Too high ..."]
        [else        "Correct! Well guessed!"]))

(define (guess-number target)
  (let ([delta (- (read) target)])
       (displayln (num-message delta))
       (zero? delta)))

(define (number-guessing-game target tries)
  (cond [(zero? tries)         (displayln "Too many attempts")]
        [(guess-number target) (displayln "You won")]
        [else                  (number-guessing-game target (- tries 1))]))

(number-guessing-game NUM TRIES)


Removing unneeded objects from an array before rendering I'm making a function that adds a new object to an array and removes the oldest if it has reached a specific length. Click the button in the example below. All objects are drawn until its array reaches a length of 5 when the oldest is removed.
Is there a better, more processor and memory efficient way to do this instead of needlessly throwing the oldest entry at the garbage collector?

let c = document.getElementById("context");
let ctx = c.getContext("2d");
let particles = [];

function addParticle() {
  particles.push({
    x: Math.random() * 800,
    y: Math.random() * 500,
    width: Math.random() * 100 + 50,
    height: Math.random() * 100 + 50,
    color: '#' + Math.floor(Math.random() * 16777215).toString(16)
  });

  if (particles.length > 5) particles.shift();

  draw();
}

function draw() {
  ctx.clearRect(0, 0, 800, 500);

  for (let i = 0; i < particles.length; i++) {
    ctx.fillStyle = particles[i].color;
    ctx.fillRect(particles[i].x, particles[i].y, particles[i].width, particles[i].height);
  }
}
<button onclick="addParticle();">Add</button>
<canvas id="context" width="800" height="500"></canvas>

I think the current code is just fine from an efficiency standpoint. The overhead of a single small object is essentially nothing. Even if 1000 such objects were created and then got GC'd every time the button was clicked, on any remotely modern device, the performance impact would almost certainly be imperceptible.
If you really wanted to be more efficient, it might be very slightly more efficient to use a rotating modulo index that gets used to assign the new object instead of re-shuffling the array indicies with unshift, but with only 5 elements in the array, it's not something worth worrying about. Better to worry about performance tweaks only if

The improvement is easy to implement and doesn't make the code harder to read, or
You've tested the script on a low-end device and know that something isn't running as fast as it should - in which case you should run a check to see what section(s) of the code are causing the bottleneck(s), and then work on improving only those bottlenecks.

IMO, anything else is premature optimization - your efforts are better spent elsewhere, such as making the code clean and readable, which usually matters far more than performance.
And there are a number of improvements you could make on the code quality front:
Use const by default - only use let when you must reassign the variable name. Using const makes the code more readable when one can identify at a glance that something isn't going to be reassigned.
Avoid inline handlers - they have a whole bunch of problems, including a demented scope chain which virtually requires global pollution, mixing page content with presentation, and ugly escaping issues. When you have an event listener you want to attach to an element, better to use addEventListener.
Abstract iteration When iterating, unless you actually need to work with the index, it's more abstract and more readable to work with the element being iterated over directly instead. For example, rather than particles[i].width, it'd be nicer to use particle.width, or even just width extracted from the particle. You can do this by invoking Array#forEach or the array's iterator.
ID You have <canvas id="context" width="800" height="500"></canvas>. But the canvas is not the 2d context - giving the element an ID of context is somewhat misleading. Also, IDs create global variables by default, unfortunately, which can sometimes lead to hard-to-understand bugs. You can select the canvas with querySelector('canvas') instead. (If you had a larger page and collisions were possible, you could give the element unique classes instead; classes do not create global identifiers.)

const button = document.querySelector('button');
button.addEventListener('click', addParticle);

const ctx = document.querySelector('canvas').getContext("2d");
const particles = [];

function addParticle() {
  particles.push({
    x: Math.random() * 800,
    y: Math.random() * 500,
    width: Math.random() * 100 + 50,
    height: Math.random() * 100 + 50,
    color: '#' + Math.floor(Math.random() * 16777215).toString(16)
  });

  if (particles.length > 5) particles.shift();

  draw();
}

function draw() {
  ctx.clearRect(0, 0, 800, 500);
  particles.forEach(({ color, x, y, width, height }) => {
    ctx.fillStyle = color;
    ctx.fillRect(x, y, width, height);
  });
}
<button>Add</button>
<canvas width="800" height="500"></canvas>


Producer consumer with threads and using boost ring buffer I have two threads, one is the producer and other is consumer. My consumer is always late (due to some costly function call, simulated in below code using sleeps) so I have used ring buffer as I can afford to loose some events.

I am looking to see if my locking looks alright and general c++ review comments.

#include <iostream>
#include <thread>
#include <chrono>
#include <vector>
#include <atomic>
#include <boost/circular_buffer.hpp>
#include <condition_variable>
#include <functional>

std::atomic<bool> mRunning;
std::mutex m_mutex;
std::condition_variable m_condVar;

class VecBuf {
    private:
    std::vector<int8_t> vec;

    public:
    VecBuf() = default;
    VecBuf(std::vector<int8_t> v)
    {
        vec = v;
    }
};

std::vector<int8_t> data{ 10, 20, 30 };

class Detacher {
    public:
    template<typename Function, typename ... Args>
    void createTask(Function &&func, Args&& ... args) {
        m_threads.emplace_back(std::forward<Function>(func), std::forward<Args>(args)...);
    }

    Detacher() = default;
    Detacher(const Detacher&) = delete;
    Detacher & operator=(const Detacher&) = delete;
    Detacher(Detacher&&) = default;
    Detacher& operator=(Detacher&&) = default;

    ~Detacher() {
        for (auto& thread : m_threads) {
            thread.join();
        }
    }

    private:
    std::vector<std::thread> m_threads;
};

void foo_1(boost::circular_buffer<VecBuf> *cb)
{
    while (mRunning) {
        std::unique_lock<std::mutex> mlock(m_mutex);

        m_condVar.wait(mlock, [=]() { return !cb->empty(); });

        VecBuf local_data(cb->front());
        cb->pop_front();
        mlock.unlock();
        if (!mRunning) {
            break;
        }
        //simulate time consuming function call and consume local_data here
        std::this_thread::sleep_for(std::chrono::milliseconds(16));
    }

    while (cb->size()) {
        VecBuf local_data(cb->front());
        cb->pop_front();
        if (!mRunning) {
            break;
        }
    }
}

void foo_2(boost::circular_buffer<VecBuf> *cb)
{
    while (mRunning) {
        std::unique_lock<std::mutex> mlock(m_mutex);

        while (cb->full()) {
            mlock.unlock();
            /* can we do better than this? */
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
            mlock.lock();
        }
        cb->push_back(VecBuf(data));
        m_condVar.notify_one();
    }
}

int main()
{
    mRunning = true;
    boost::circular_buffer<VecBuf> cb(100);
    Detacher thread_1;
    thread_1.createTask(foo_1, &cb);
    Detacher thread_2;
    thread_2.createTask(foo_2, &cb);
    std::this_thread::sleep_for(std::chrono::milliseconds(20000));
    mRunning = false;
}

/* can we do better than this? */

In a circular buffer context, the standard way to avoid busy waiting is to have two semaphores. First to block a producer when a buffer is full, and second to block a consumer when the buffer is empty. Once a process passes its semaphore, and does its job, it should signal the peer.

The circular buffer is good when the consumer is only sometimes late and you cannot afford loosing data. In your situation it looks like a wrong solution: the producer becomes throttled by the rate of consumption, and the consumer is a presented with the stale data.

A typical answer is to let the producer run at full speed, and triple-buffer the production (at least, it guarantees that the consumer would get the most recently produced data). Please forgive the shameless self promotion.


Casting to less generic types The source code, or rather the concept I'd like to get reviewed is what now allows me to do the following during reflection:

object validator;  // An object known to implement IValidation<T>
object toValidate; // The object which can be validated by using the validator.

// Assume validator is IValidation<string> and toValidate string.

IValidation<object> validation
    = Proxy.CreateGenericInterfaceWrapper<IValidation<object>>( validator );

validation.IsValid( toValidate ); // This works! No need to know about the type.

// Assuming the validator validates strings, this will throw an InvalidCastException.
//validation.IsValid( 10 );

I no longer need to know about concrete types during reflection where I know types will match. It looks kind of like covariance but the result only works because types are guaranteed to match. This allows me to support any type, since no manual type checks need to be done to cast to the correct interface type.

The CreateGenericInterfaceWrapper method uses RunSharp to emit the code. This source code, and an extended explanation can be found on my blog.

My main concerns are:

Did anybody encounter such a use case before, and found a better way to solve it?
Did I write something which I could just have found in an existing library?

To further clarify why I wrote this solution, I copied the example from my blog of the code I would have to write to be able to use the IsValid method during reflection.

object validator;  // An object known to implement IValidation<T>
object toValidate; // The object which can be validated by using the validator.

// 'validator' can be any type validator which can validate 'toValidate'.

if ( validator is IValidation<string> )
{
    IValidation<string> validation = (IValidation<string>)validator;
    validation.IsValid( (string)toValidate );
}
else if ( validator is IValidation<int> )
{
    IValidation<int> validation = (IValidation<int>)validator;
    validation.IsValid( (int)toValidate );
}
else if ... // and so on to support any possible type ...

Did anybody encounter such a use case before, and found a better way to solve it?

There's the IEnumerable pattern:

public interface IValidation
{
    bool IsValid(object obj);
}

public interface IValidation<T> : IValidation
{
    bool IsValid(T t);
}

public abstract class BaseValidation<T> : IValidation<T>
{
    public bool IsValid(object obj)
    {
        return IsValid((T)obj);
    }

    public abstract bool IsValid(T t);
}

Whether it's better or not is debatable, but it should at least be familiar to anyone who has to maintain your code.


Dynamically adding QLineEdits and then copy text from them to vector in right order Here is how it looks at the beginning:

And after I can add random counts of line edits:

Or delete them:

The main purpose is the possibility to add or delete QlineEdits which stores user names and each QlineEdit adds a username to the vector in the right order (index) like this:

users =>
 [0] = "a";
 [1] = "b";
 [2] = "c";
 [3] = "d";
 [4] = "e";
 [5] = "f";
 [6] = "g";

Here is the code :
mainwindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <QVBoxLayout>
#include <QHBoxLayout>
#include <QLineEdit>
#include <QPushButton>
#include <QList>
#include <vector>
class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

private slots:
    void addbutton_Clicked();
    void deletebutton_Clicked();
    void lineedit_textChanged(QString text);

private:
    QVBoxLayout *middlelayout;
    QList<QLineEdit*> qlist;
    std::vector<QString> users;
};

#endif // MAINWINDOW_H

mainwindow.cpp

#include "mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent)
{
    QWidget window = new QWidget(this);
    QVBoxLayout *mainlayout = new QVBoxLayout;
    middlelayout = new QVBoxLayout;
    QHBoxLayout *buttonlayout = new QHBoxLayout;
    QPushButton *addbutton = new QPushButton;
    QPushButton *deletebutton = new QPushButton;

    addbutton->setText("ADD");
    deletebutton->setText("Delete");

    buttonlayout->addWidget(addbutton);
    buttonlayout->addWidget(deletebutton);
    buttonlayout->setAlignment(Qt::AlignTop);
    mainlayout->addLayout(buttonlayout);
    mainlayout->addLayout(middlelayout);
    window->setLayout(mainlayout);
    setCentralWidget(window);

    connect(addbutton, SIGNAL(clicked()), SLOT(addbutton_Clicked()));
    connect(deletebutton, SIGNAL(clicked()), SLOT(deletebutton_Clicked()));
}

MainWindow::~MainWindow()
{
}

void MainWindow::addbutton_Clicked()
{
    QLineEdit *lineedit = new QLineEdit;
    qlist.push_back(lineedit);
    middlelayout->addWidget(lineedit);
    lineedit->setObjectName("lineedit"+QString::number(qlist.count()));

    users.resize(qlist.count());
    connect(lineedit, SIGNAL(textChanged(QString)), SLOT(lineedit_textChanged(QString)));
}

void MainWindow::deletebutton_Clicked()
{
    QWidget *child;
    if(!middlelayout->isEmpty())
        if((child = middlelayout->takeAt(qlist.count()-1)->widget()) != 0) {
            delete child;
            qlist.removeLast();
    }
}

void MainWindow::lineedit_textChanged(QString text)
{
    unsigned int index = 0;
    QObject *obj = QObject::sender();
    QString senderobjName = obj->objectName();
    int isTextchanged = senderobjName.indexOf("lineedit") != -1;

    if(isTextchanged)
    {
        QStringList list = senderobjName.split("lineedit");
        index = list[1].toInt();
        users[index]=text;
    }
}

The code works fine but it seems that I'm doing it wrong and there is a better way.

Since the MainWindow destructor is empty, you can just omit it.

In lineedit_textChanged, you declare the index variable at the top,
but it's only used if the text has changed.
It would be better to declare it inside the if branch where you actually use it:

if (isTextchanged)
{
    QStringList list = senderobjName.split("lineedit");
    unsigned int index = list[1].toInt();
    users[index] = text;
}

This snippet can be a bit confusing without braces on the outer if:

if(!middlelayout->isEmpty())
    if((child = middlelayout->takeAt(qlist.count()-1)->widget()) != 0) {
        delete child;
        qlist.removeLast();
}

It would be clearer this way:

if (!middlelayout->isEmpty()) {
    if ((child = middlelayout->takeAt(qlist.count()-1)->widget()) != 0) {
        delete child;
        qlist.removeLast();
    }
}


A simple implementation of the principle of RSA encryption Inspired by a Numberphile video I made a little program that shows the principles of RSA encryption and decryption. To calculate the keys I used the explanation in this link: rsa public private key encryption explained. Fascinating what you can do in a few lines of code and how Python can handle to powering of large numbers.

One observation is that with large prime numbers encryption goes relatively fast starting with ascii code numbers that are relatively small less than 200 or so, but the decryption goes much slower as the encrypted numbers are magnitudes larger. How is this solved in practice? and how is this all working with really large primes?

Comments, suggestions welcome.

'''  RSA encryption
     inspired on: https://www.youtube.com/watch?v=M7kEpw1tn50
     and http://jcla1.com/blog/rsa-public-private-key-encryption-explained

     some conditions:
        - prime numbers must be > 1 and not equal
        - prime factor must sufficiently large to accommodate the ascii numbers, let's say > 150
        - so for example (2, 191) will do as well as (11, 17)
'''

class RSA():
    ''' methods for calculating keys, encrypt and decrypt ascii messages
    '''
    @staticmethod
    def gcd(a, b):
        while b:
            a, b = b, a % b
        return a

    @classmethod
    def encrypt(cls, message):
        message_letters = [ord(letter) for letter in message]
        message_encrypted = ''.join([chr(letter**cls.public_key % cls.prime_factor) for letter in message_letters])
        return message_encrypted

    @classmethod
    def decrypt(cls, message_encrypted):
        message_encrypted_letters = [ord(letter) for letter in message_encrypted]
        message = ''.join([chr(letter**cls.private_key % cls.prime_factor) for letter in message_encrypted_letters])
        return message

    @classmethod
    def calc_keys(cls, prime_1, prime_2):
        cls.prime_factor = prime_1 * prime_2
        totient = (prime_1 - 1) * (prime_2 - 1)

        # calculate the possible public keys where gcd(public_key, totient) == 1, then select the 5th one (this is abritary, any
        # of the public_keys could have been selected
        # (Note above link has an error that the gcd of public_key and totient must be 1, not public_key
        #  and the prime_factor as suggested in the article)
        public_keys = []
        for i in range(totient):
            if cls.gcd(i, totient) == 1:
                public_keys.append(i)
        cls.public_key = public_keys[4]

        # calculate the private key based on public key and totient when (public_key * private_key - 1) % totient == 0
        cls.private_key = 0
        x = -1
        while x != 0:
            cls.private_key += 1
            x = (cls.public_key * cls.private_key - 1) % totient

        return (cls.prime_factor, cls.public_key, cls.private_key)

def main():
    rsa = RSA()
    print(rsa.calc_keys(61, 53))

    message = 'hello this is my encrypted message'
    encrypted_message = rsa.encrypt(message)
    decrypted_message = rsa.decrypt(encrypted_message)

    if message == decrypted_message:
        print('Hurray!!')
        print(f'message: {message}\nencrypted message: {encrypted_message}'
              f'\ndecrypted message: {decrypted_message}')
    else:
        print('Ough, someting wrong here  ... !')

if __name__=="__main__":
    main()

1) Real implementation of RSA use the Chinese Remainder Theorem, which greatly improves the performance.

2) The big performance difference between encryption and decryption is a normal thing for RSA. It comes from the fact, that the performance of the modular exponentiation used depends on the number of 1 bits in the exponent. If you either chose the public exponent to be very small (like you are doing in your code, which is insecure btw) or large but with a lot of zeroes in its binary representation (usually e=655537=100...001 is used) public key operations (=encryption) will be fast. The private exponent can not be controlled and will have ~1/2 of its bit set to one, making the decryption a lot slower.


Checking if a number is a perfect square by recursive sum of its digits I was solving this question on an online judge.

The problem boils down to the following:

  Given a number \$n\$, where \$1 \le n \le 10^{18}\$, we need to check if it contains odd number of divisors.

I have used following facts in my code:

only perfect squares have an odd number of divisors
the recursive sum of digits of perfect squares is always 1,4,7 or 9

A possible solution is:

Find the recursive sum of the digits of the given number
Check if the recursive sum is 1 or 4 or 7 or 9. ( as the recursive sum of digits of perfect squares is always 1,4,7 or 9 )

Here is my code for it.  It gives the correct answer for small values, but when submitted to the online judge, it shows "Wrong Answer", which means there is some bug in the code.
Can anyone explain why this is happening ?

#include<bits/stdc++.h>
using namespace std;

 long long recursiveSum(long long num){
 long long sum = 0;
 while(num != 0){
 sum = sum + num%10;
 num = num/10;
 }
 if(sum/10 != 0){
    return recursiveSum(sum);
 }
 else{
     return sum;
 }

 }

int main()
{
long long t,r;
long long n;
cin>>t;     //   t is the number of test cases
for(long long p=1;p<=t;p++)
{
    cin>>n;

    r=recursiveSum(n);

    if(r==1 || r==4 || r==7 || r==9) cout<<"Case "<<p<<": Yes\n";

    else cout<<"Case "<<p<<": No\n";

}

}

Your code works, as specified, but your specification is wrong.

Sometimes doing a code review involves fact-checking. In this case, your facts are wrong. What looked fishy to me, is the statement that:

  the recursive sum of digits of perfect squares is always 1,4,7 or 9

(well, I also checked that only perfect squares have an odd number of divisors - which is true).

What is the truth about the sum of digits? Well, it is true that all perfect squares have recursive digit sums that total 1, 4, 7, or 9, but the inverse is not true. This is obvious if you take the value 7, which is not a perfect square, but has the digit sum of 7.

When I looked around, I found this entertaining read: How to check if a number is a Perfect Square. It is quite clear that this is the second line of attack for checking whether a number is a perfect square. The first line is to check the last digit (which excludes 7, by the way).

Even once you have done (and passed) these two tests, all you can say is that the number is likely a perfect square. You still need to do the hard work to confirm it.

As an aside... why do you use recursion for the digit-sum. Something iterative would be great:

 long long iterativeSum(long long num){
     while(num >= 10){
        long long sum = 0;
        while (num > 0) {
            sum += num % 10;
            num /= 10;
        }
        num = sum;
     }
     return num;
  }


Parsing a tabular log file into a hashmap I have this method that reads a file into a hashmap. I feel like there are too many levels in this code with all these tries, ifs and so on. How do you think this can be improved?

public static ExchangeHistory loadHistory(String path){
    Map<Integer, Date> rtcHistory = new HashMap<Integer, Date>();
    Map<Integer, Date> almHistory = new HashMap<Integer, Date>();

    ExchangeHistory exchHist = new ExchangeHistory(rtcHistory, almHistory);

    File file = new File(path);

    BufferedReader reader = null;
    try{
        reader = new BufferedReader(new InputStreamReader(new FileInputStream(file)));

        String line;
        int lineNumber = 1;

        // read file line by line
        while ((line = reader.readLine()) != null){
            String[] tokens = line.split("\\s");
            if (tokens.length != 3){
                log.error("Unable to parse exchange history file at line: " + lineNumber);
                continue;
            }

            // try to parse line. if parsing fails, just skip it
            try {
                int id = Integer.parseInt(tokens[1]);
                long timeInMillis = Long.parseLong(tokens[2]);

                if (tokens[0].equals(System.RTC)){
                    rtcHistory.put(id, new Date(timeInMillis));
                } else if (tokens[0].equals(System.ALM)){
                    almHistory.put(id, new Date(timeInMillis));
                }

            } catch (Exception e){
                log.error("Unable to parse exchange history file at line:" + lineNumber, e);
            }

            lineNumber++;
        }

    } catch (FileNotFoundException e){  // try to create a new file in case it doesn't exist

        log.warn("No exchange history file found at " + path);

        try {
            file.createNewFile();
            log.info("Created new exchange history file at "  + path);
        } catch (IOException ioe){
            log.error("Unable to create exchange history file at " + path
                    + ". Exchange session info won't be saved", ioe);
        }
        // return an empty history list cause there are no records in new file
        return exchHist;
    } catch (IOException e) {
        log.error("Unable to read exchange history file at " + path
                + ". All items will be synchronized anew", e);
        return exchHist;
    } finally{
        if (reader != null){
            try {
                reader.close();
            } catch (IOException e) {
                log.error("Unable to close input stream. Resource leak possible", e);
            }
        }
    }

    return exchHist = new ExchangeHistory(rtcHistory, almHistory);
}

This is indeed way too complex. In fact I think there's enough responsibilities in this one method to be spread over a few classes.

managing the history file
parsing history records
processing history records
counting lines
logging

These points may map to simply a helper method or a separate class.

Try organizing this behavior in several levels of abstraction. Try checking the existence of the file prior to tryig to parse its contents. Making a class that models the content of one record will definitely help. It'll make a good home for some of the validation and parsing logic. Don't be afraid to make private helper methods. They can greatly improve readability and lower complexity per method significantly.

The top level version of this method could look like this :

public ExchangeHistory loadHistory(String path) {
    List<HistoryRecord> records = new ArrayList<>();
    for (String line : new LineIterable(getHistoryReader(path))) {
        records.add(HistoryRecord.fromTokenizedString(line));
    }
    return ExchangeHistory.fromHistoryRecords(nonNulls(records));
}

In which

getHistoryReader(path) will fetch or create the file and wrap it as a reader.
LineIterable abstracts reading the BufferedReader to an Iterable (enabling the advanced for loop)
HistoryRecord parses the tokens of a line, and can be queried for the parsed data.

fromTokenizedString(line) returns a valid HistoryRecord or null

nonNulls() filters nulls out of a List
ExchangeHistory.fromHistoryRecords() creates the ExchangeHistory based on a List/Iterable of HistoryRecords

As for logging, it always clutters code. Your error logging currently seems to be a placeholder for decent exception handling. The other logging appears to be used for debugging. I like AOP to do logging, since it allows you to focus on the code, and the logging can be coded separately.


Optimal way to Compare two arrays that are widely different I have two different arrays that are widely different in length, arrayA, arrayB

for example:arrayA.Length=1000  and arrayB.Length=100,000

Create a boolean function that returns true if each item in arrayA are also  present in arrayB, at least the same number of times as in arrayA.

Please review my code below and help me with siuggestions to improve this code below

    private static bool IsMinRequiredFrequencyMatch(int[] arrA,int[] arrB)
    {
    bool result = false;
    Array.Sort(arrA);

    //create dictionary from shorter array
    var dict = new Dictionary<int, int>();
    for (int i = 0; i < arrA.Length; i++)
    {
        var ch = arrA[i];
        if (!dict.ContainsKey(ch))
        {
            dict[ch] = 1; //add char count
        }
        else
        {
            dict[ch]++; //increase count
        }
    }

    //loop through Dictionary
    foreach (var kvp in dict)
    {
        var chr = kvp.Key; //target item to find
        var expectedcount = kvp.Value; //expected count of item in arrB
        int actualCount = 0; //actual count of item in arrB

        //loop through arrayB
        for (int i = 0; i < arrB.Length; i++)
        {
            if (arrB[i] == chr)
            {
                actualCount++;
            }

            if (actualCount >= expectedcount)
            {
                result = true;
                break;
            }
        }

        if (actualCount < expectedcount)
        {
            result = false;
            break;
        }
    }

    return result;

}

Sure, you can always write your own method for everything but if you are lazy you could just use LINQ with ToLookup + All + Count

var arr1 = new[] { 1, 2, 3, 4, 3, 2 };
var arr2 = new[] { 6, 7, 3, 4, 1, 2, 2, 3 };

var lookup1 = arr1.ToLookup(x => x);
var lookup2 = arr2.ToLookup(x => x);

var isFullyContained = lookup1.All(x => lookup2[x.Key].Count() == x.Count());

false - means arr1 is fully contained in arr2

The good thing about lookup is that you don't need the ContainsKey method because if the key does not exist it'll return an empty collection.

Review

You create a dictionary only for the first array but you need one for the second array too or otherwise you need to loop over it each time which is unnecessary. Just count the items and compare their counts.


Java Array Traversal Optimization I'm currently learning JAVA in a class and we're using the ACM Graphics package. I wanted some pointers on my code. To provide some context here is the assignment:

  Write a program that creates an array of 5 custom objects (cows, UFOs,
  cars, etc.) and then lines them up along the left edge of the window.
  Assign each object a random velocity, and then move the objects to the
  right edge of the window. When one of the objects (the "winner")
  reaches the right edge of the window, remove the other objects and
  cause the winner to flash on and off.

This works just fine and fulfills all the requirements from the assignment. What I want to know is how can I improve this code while still remaining within the requirements. Also any reading materials/videos that you think would be useful for my learning please feel free to share them.

Main Race file:

import acm.graphics.*;
import acm.program.*;
import acm.util.RandomGenerator;
import java.awt.Color;

public class Race extends GraphicsProgram {

    //Setting up variables
    private static final int WAIT = 20;
    private static final int WINDOWX = 400;
    private static final int WINDOWY = 600;
    private static final int UFO_COUNT = 5;
    private UFO[] ufos = new UFO[UFO_COUNT];
    private boolean flag = true;
    private RandomGenerator rgen = new RandomGenerator();

    public void init() {
        setSize(WINDOWX, WINDOWY);
    }

    public void run() {
        //Set up UFOs at start
        for (int i=0; i<UFO_COUNT; i++) {
            ufos[i] = new UFO(0, i*100);
            ufos[i].alien.setColor(rgen.nextColor());
            add(ufos[i]);
        }

        int winner = -1;
        waitForClick();

        while(flag) {
            pause(WAIT);

            //Move Ufos
            for (int i=0; i<ufos.length; i++ ) {
                ufos[i].move(rgen.nextInt(0, 2), 0);

                //Is there a winner?
                if(ufos[i].getX() > WINDOWX){
                    winner = i;
                    flag = false;
                }
            }
        }

        if(!flag) //Safeguard
            displayWinner(winner);
    }

    private void displayWinner(int winner) {
        //Hide others
        for (int i=0; i<ufos.length; i++ ) {
            ufos[i].setVisible(false);
        }

        //Make the winner flash
        while(true) {
            ufos[winner].setVisible(false);
            pause(200);
            ufos[winner].setVisible(true);
            pause(200);
        }
    }
}

UFO Constructor Class:

import acm.graphics.*;
import java.awt.*;

public class UFO extends GCompound {
    public GOval body, bubble, alien;

    public UFO(int x, int y){
        //Draw ufo
        body = new GOval(100, 50);
        body.setFilled(true);
        body.setColor(Color.RED);

        bubble = new GOval(50, 50);
        alien = new GOval(20, 20);
        alien.setFilled(true);
        alien.setColor(Color.GREEN);

        add(bubble, x+26, y+0);
        add(alien, x+40, y+10);
        add(body, x+0, y+26);
    }
}

Encapsulation

An object's behaviour should be with the object. This means that the UFO should control how it flashes (or blinks):

public class UFO ...
  public void blink() {
    setVisible(false);
    pause(200);
    setVisible(true);
    pause(200);
  }

  /** You will have to implement and test this method. */
  private void pause( int milliseconds ) {
    Thread.sleep( milliseconds);
  }
}

Information Hiding

The following code exposes the internals of how a UFO is piloted:

ufos[i].alien.setColor(rgen.nextColor());

Rather, this should be exposed as an accessor:

ufos[i].setPilotColor(rgen.nextColor());

Similarly, exposing the UFO's location is not necessary to determine the winner:

if(ufos[i].getX() > WINDOWX){
  winner = i;
  flag = false;
}

Rather, the above code is more clearly stated as:

if(ufos[i].crossedFinishLine( WINDOWX ) ){
  winner = i;
  flag = false;
}

Only the UFO need know if it has crossed the finish line. The code is now self-documenting.

Make the following attributes private:

public GOval body, bubble, alien;

Otherwise your code will be susceptible to race conditions (no pun intended) and be difficult to debug. This is because the UFO class has no control over when its data changes.

While True

In my opinion, while( true ) is poor form. Instead, add a condition that allows the loop to exit:

    while( gameOver() ) {
        ufos[winner].blink();
    }


Fetching data from a MySQL database using Promise I have written JavaScript code for fetching data from a MySQL database. I have used connection pooling and Promise API.

var mySql = require("mysql");
var Promise = require("promise");

var pool = mySql.createPool({
  host : "localhost",
  user : "root",
  password : "rahul",
  database : "testDb", //schema
  connectionLimit : 13, // at a time 13 connection be created in pool
});

function getDepartments(){
  return new Promise(fn);

  function fn(resolve,reject){
    pool.getConnection(function(err,con){
        if(err){
            return reject(err);
        }else{
            con.query("select * from departmentTbl",function(err,rows){
                if(err){
                    return reject(err);
                }else{
                    con.release(); // releasing connection to pool
                    return resolve(rows);
                }
            });
        }
      }); // getConnection
    }// fn
 } // getDepartments

getDepartments().then(function(rows){
  console.log(rows);
}).catch(function(err){
  console.log(err);
}).done(function(){
  pool.end();   // closing all connections in pool
});

Can I further improve the code? I have only used one Promise. Can I use multiple promises to avoid callback hell?

Resource leakage: put con.release() outside the else block.


Is there a more succinct way of writing this simple JavaScript loop? This seems a bit redundant to me, but I'm not sure of how else I might be able to write this.  Normally I'd use a switch statement, but I don't think that'll work here.  I realize I could also do this in a for loop, but the main heart of the question is what's inside the loop itself.  "arr" is an array, just in case it's not obvious.

var i = 0, x = arr.length;

while (i < x) {
    if (typeof arr[i] === 'object' && !Array.isArray(arr[i])) {
        attr = arr[i];
    }
    else if (typeof arr[i] === 'string') {
        text = arr[i];
    }
    else if (typeof arr[i] === 'object' && Array.isArray(arr[i])) {
        child = arr[i];
    }
    i++;
}

I think at the very least I'd rearrange the cases to make the dichotomy between Array.isArray and !Array.isArray more direct and apparent:

while (i < x) {
    if (typeof arr[i] === 'string') {
        text = arr[i];
    }
    else if (typeof arr[i] === 'object') {
        if (Array.isArray(arr[i])) {
            child = arr[i];
        }
        else {
            attr = arr[i];
        }
    }
    i++;
}

That's not really a lot shorter but I think it makes the intent rather more clear (at least assuming I've divined the real intent correctly).


Eliminate duplication in similar fake data access classes Can I combine this two classes and have one fake db class?

public class FakeDb : Dictionary<string, List<string>>
{
    public const string Table1 = "Table1";

    public void AddToTable(string tableName, string line)
    {
        if(!ContainsKey(tableName))
            Add(tableName, new List<string>());

        List<string> rows = GetTableData(tableName);
        rows.Add(line);
    }

    internal List<string> GetTableData(string tableName)
    {
        return this[tableName];
    }
}

public class FakeComplexDb : Dictionary<string, List<Dictionary<string,List<string>>>>
{
    public void AddToTable(string tableName, Dictionary<string, List<string>> line)
    {
        if (!ContainsKey(tableName))
            Add(tableName, new List<Dictionary<string, List<string>>>());

        List<Dictionary<string, List<string>>> rows = GetTableData(tableName);
        rows.Add(line);
    }

    internal List<Dictionary<string, List<string>>> GetTableData(string tableName)
    {
        return this[tableName];
    }
}

Why not use generics:

public class FakeDb<TList> : Dictionary<string, List<TList>>
{
    public const string Table1 = "Table1";

    public void AddToTable(string tableName, TList line)
    {
        if (!ContainsKey(tableName))
        {
            Add(tableName, new List<TList>());
        }

        List<TList> rows = GetTableData(tableName);
        rows.Add(line);
    }

    private List<TList> GetTableData(string tableName)
    {
        return this[tableName];
    }
}

Then to use:

var simpleDB = new FakeDb<string>();
var complexDb = new FakeDb<Dictionary<string, List<string>>>();


Executor Service with Blocking Queue I did not find a single working implementation of using an Executor Service with a Blocking Queue. So , I have come with with a working sample. Thoughts?

import java.util.Map;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;

class Producer implements Runnable {

    protected BlockingQueue queue = null;

    public Producer(BlockingQueue queue) {
        this.queue = queue;
    }

    public void run() {
        try {

            for (int i = 0; i < 10000; i++) {
                queue.put(i);
                // System.out.println("Producer ID "+Thread.currentThread().getId()+" is adding task : "+i);
            }
            // poison pill
            // System.out.println("Poison number added! "+Thread.currentThread().getId());
            queue.put(-1);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

class Consumer implements Runnable {

    protected BlockingQueue queue = null;
    ConcurrentHashMap<Integer, Integer> hashmap;

    public Consumer(BlockingQueue queue,
            ConcurrentHashMap<Integer, Integer> hashmap) {
        this.queue = queue;
        this.hashmap = hashmap;
    }

    public void run() {
        try {

            while (ExecutorTest.isRunning) {
                Integer i = (Integer) queue.take();
                System.out.println("Consumer " + Thread.currentThread().getId()
                        + ": taking Task : " + i);
                if (i == -1) {
                    queue.put(i);
                    ExecutorTest.isRunning = false;
                    // System.out.println("Setting isRunning to false :  "+Thread.currentThread().getId());
                    break;
                }
                hashmap.put(i, i);
            }

        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

public class ExecutorTest {

    static BlockingQueue<Integer> queue = new LinkedBlockingQueue<Integer>();
    static ConcurrentHashMap<Integer, Integer> hashmap = new ConcurrentHashMap<Integer, Integer>();
    static volatile boolean isRunning = true;

    public static void main(String[] args) {
        Producer producer = new Producer(queue);
        Consumer consumer = new Consumer(queue, hashmap);

        new Thread(producer).start();
        ExecutorService executorService = Executors.newFixedThreadPool(4);

        executorService.execute(consumer);
        executorService.execute(consumer);
        executorService.execute(consumer);
        executorService.execute(consumer);

        // wait for the threads to finish
        while (isRunning) {
        };

        executorService.shutdown();
        System.out.println("SHUT DOWN THREAD POOL");

        try {
            executorService.awaitTermination(10, TimeUnit.SECONDS);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("HASHMAP SIZE : " + hashmap.size());
    }

    public static void printHashMap() {
        for (Map.Entry<Integer, Integer> map : hashmap.entrySet()) {
            System.out.println("Entry " + map.getKey() + ":" + map.getValue());
        }
    }

}

Producer

protected BlockingQueue queue = null;

Why not apply generics? BlockingQueue<Integer> avoids awkard casting later on.

Why is this protected? Fields are typically private, and there doesn't seem any motivation in your example to widen visibility on the field itself

} catch (InterruptedException e) {
    e.printStackTrace();
}

This is not the proper way to handle an InterruptedException. The ExecutorService uses interruption for forced shutdown (shutDownNow()), but since you eat the interruption the ExecutorService will not be responsive to forced shotdown. In general you would always propagate interruption to the caller.

Consumer

Actually the remarks for Producer go for Consumer as well.

but also

while (ExecutorTest.isRunning) {

Alternatively you could simply use interruption to stop the task : while (!Thread.currentThread().isInterrupted()).
But what's worse is that isRunning is actually a field on ExecutorTest, This means Consumer class depends on ExecutorTest class for its proper working.

ExecutorTest

ExecutorService executorService = Executors.newFixedThreadPool(4);

executorService.execute(consumer);
executorService.execute(consumer);
executorService.execute(consumer);
executorService.execute(consumer);

Perhaps a small for loop, and make the literal 4 variable or a constant.

// wait for the threads to finish
while (isRunning) {
};

This is unnecessary, you already do executorService.awaitTermination(10, TimeUnit.SECONDS);
A busy wait like this is also a poorer way to await some other thread, as you are wasting CPU cycles.


Recording all instances of a class - Python In a programme that I am writing, I need to loop through all the instances of a class, the way I got around this was by appending every class to a list like below:

allPeople = []
class Person:

    def __init__(self, name, age, height):
        self.name = name
        self.age = age
        self.height = height
        allPeople.append(self)

Jeff = Person("Jeff", 20, "1.6")
Bob = Person("Bob", 39, "1.4")
Helen = Person("Helen", 19, "1.3")

for person in allPeople:
    print(person.name + " is " + str(person.age))

Is this the best way of doing things or is there a built in python function for this?

Your approach is ok, but I have just one mention (this is based on personal preferences): I'd iterate over a class not a list which is defined outside it. You'll just have to define a metaclass which supports iteration.

class IterPerson(type):
    def __iter__(cls):
        return iter(cls._allPeople)

class Person(metaclass=IterPerson):
    _allPeople = []

    def __init__(self, name, age, height):
        self._allPeople.append(self)

        self.name = name
        self.age = age
        self.height = height

if __name__ == '__main__':
    Jeff = Person("Jeff", 20, "1.6")
    Bob = Person("Bob", 39, "1.4")
    Helen = Person("Helen", 19, "1.3")

    for person in Person:
        print(person.name + " is " + str(person.age))

Magic methods are always looked up on the class, so adding __iter__ to the class won't make it iterable. However the class is an instance of its metaclass, so that is the correct place to define the __init__ method.

Please note that I also put a _ in front of your list. This is how we tell python the list we'll create is private. I have also used if __name__ == "main": which is good practice (it is used to execute some code only if the file was run directly, and not imported)

Second, keep this kind of thing as simple as possible. Don't waste a lot of time and energy on something complex. This is a simple problem, keep the code as simple as possible to get the job done.


Find sequence to a target number using restricted set of primitive operations Given an integer , compute the minimum number of operations(+1, x2, x3) needed
to obtain the number  starting from the number 1.

I did this using this code:
#include<iostream>
#include<algorithm>
#include<vector>
using namespace std;
int main()
{
    int n;
    cin >> n;
    vector<int> v(n+1, 0);
    v[1] = 1;
    for(int i = 1; i < v.size(); i++)
    {
        if((v[i + 1] == 0) || (v[i + 1] > v[i] + 1))
        {
            v[i + 1] = v[i] + 1;
        }
        if((2*i <= n) && (v[2*i] == 0 || v[2*i] > v[i] + 1))
        {
            v[2*i] = v[i] + 1;
        }
        if((3*i <= n) && (v[3*i] == 0 || v[3*i] > v[i] + 1))
        {
            v[3*i] = v[i] + 1;
        }
    }
    cout << v[n] - 1 << endl;
    vector<int> solution;
    while(n > 1)
    {
        solution.push_back(n);
        if(v[n - 1] == v[n] - 1)
        {
            n = n-1;
        }
        else if(n%2 == 0 && v[n/2] == v[n] - 1)
        {
            n = n/2;
        }
        else if(n%3 == 0 && v[n/3] == v[n] - 1)
        {
            n = n/3;
        }
    }
    solution.push_back(1);
    reverse(solution.begin(), solution.end());
    for(size_t k = 0; k < solution.size(); k++)
    {
        cout << solution[k] << ' ';
    }
}

Input:
5

Output:
3
1 2 4 5

Do you have any optimized way to do this?

using namespace std;
Stop doing this.  It is a easy, but sloppy, way to code; a change to the standard library that introduces a new identifier can break your code.
Being explicit, and writing std::vector instead of vector everywhere would be painful.  But there is a middle ground:
#include <vector>
using std::vector;

Now you can lazily use vector, without fear that something you are not using from the standard library will suddenly become defined, colliding with your identifiers, and causing carnage.
White space
Either put white space around all binary operators, like v[i + 1], or never put the white space around the binary operators, like v[i*2].  But be consistent.
cout << endl;
Don't use this; it slows your code down.  The endl manipulator does two things: it adds \n to the stream AND it flushes the stream.  If you don't need to flush the stream (and you rarely do), simply write
cout << '\n';

Avoid repeated calls to functions that return the same result
for(int i = 1; i < v.size(); i++)

What is the value of v.size()?  Will it ever change?  Can the compiler tell it won't, and optimize it out?  Could you store the value in a local variable to avoid the repeated function calls?
Or ... you could use the variable that already exists: n.
for(int i = 1; i <= n; i++)

Don't Repeat Yourself (DRY)
        if((v[i + 1] == 0) || (v[i + 1] > v[i] + 1))
        {
            v[i + 1] = v[i] + 1;
        }
        if((2*i <= n) && (v[2*i] == 0 || v[2*i] > v[i] + 1))
        {
            v[2*i] = v[i] + 1;
        }
        if((3*i <= n) && (v[3*i] == 0 || v[3*i] > v[i] + 1))
        {
            v[3*i] = v[i] + 1;
        }

These statements look very similar.
        if((target <= n) && (v[target] == 0 || v[target] > v[i] + 1))
        {
            v[target] = v[i] + 1;
        }

You could pull them out into a function:
inline void explore_step(vector<int> &v, int n, int i, int target) {
    if ((target <= n) && (v[target] == 0 || v[target] > v[i] + 1)) {
        v[target] = v[i] + 1;
    }
}

And then write:
        explore_step(v, n, i, i+1);
        explore_step(v, n, i, i*2);
        explore_step(v, n, i, i*3);

Optimization
You approach takes \$O(n)\$ time, because you explore each value from 1 to n.
You do this, because you don't know which values are going to be useful in reaching the target value, and test things like v[2*i] > v[i] + 1 because you don't know which values could be reached via a faster path.
A slightly better approach:

seed 1 into a list of values to explore
for each value in the list of values to explore:

for each of the 3 target values i+1, i*2, & i*3 if <= n:

if v[target] == 0, then

store v[target] = i
add target to the list of values to explore
if target == n, stop

Consider n = 10.
explore = [1], value = 1, targets = [2, -, 3]
explore = [1, 2, 3], value = 2, targets = [-, 4, 6]
explore = [1, 2, 3, 4, 6], value = 3, targets = [-, -, 9]
explore = [1, 2, 3, 4, 6, 9], value = 4, targets = [5, 8, -]
explore = [1, 2, 3, 4, 6, 9, 5, 8], value = 6, targets = [7, -, -]
explore = [1, 2, 3, 4, 6, 9, 5, 8, 7], value = 9, targets = [10, -, -]

You could use a queue for explore, but a vector of length n, and just walking forward through the items works fine.
Notice that all values reachable after 1 step [2, 3] are processed before values reachable after 2 steps [4, 6, 9], and would be processed before those values reachable after 3 steps [5, 8, 7], and so on.
More over, we've built up a trail of breadcrumbs for the fastest path.
v[10] = 9
v[9] = 3
v[3] = 1

So no searching is required to find the correct path.
Implementation left to student.

Can we do better?  What if we started with n, and explored n-1, n/2, and n/3?  An odd value can't lead to an n/2 point, and a non-multiple-of-3 can't lead to a n/3 point, so you may be pruning more values out of the search, so might be slightly faster.
[28] -> [27, 14]
     -> [26, 9, 13, 7]
     -> [25, 13, 8, 3, 12, 6]
     -> [24, 12, 4, 2, 1!, ....]


Finding the second highest version using bash array script and clean up files The goal is to print the highest version and second highest version of the list of files. The version is based on the first three digit places of the version. For example, version 2.3.0.1 version is just 2.3.0 (ignoring the last digit).

Once it can print the highest version and the second highest version of the list, it should remove all other versions which will essentially clean up the folders in that location by only keeping the current and last version.

The EXAMPLE folders that I created are:

AAA_6.6.4.12.TEST
AAA_7.6.4.12.TEST
AAA_75.6.4.12.TEST
AAA_75.7.4.12.TEST
CCC_81.0.0.0.TEST
CCC_81.2.0.0.TEST
CCC_81.2.3.0.TEST
DDD_1.0.0.0.TEST
DDD_1.0.0.1.TEST
DDD_1.0.0.6.TEST
DDD_1.1.0.0.TEST
DDD_2.0.0.0.TEST
DDD_2.0.0.1.TEST
DDD_2.0.0.3.TEST
DDD_3.0.0.0.TEST

This is the array that I have to compute the highest and second highest version:

new_var=( $(for arr in "${var[@]}"
do
    echo $arr
done | sort) )

for folder in *
do
    if [[ $folder =~ ([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})\.[0-9]{1,3} ]]
    then
        if [[ "$new_var" < "${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}" ]]
        then
            new_var="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}.${BASH_REMATCH[3]}"
        fi
    else
        echo "failed"
    fi

done;

echo "The highest version is: $new_var"
echo "The second highest version is: ${new_var[-2]}"

This prints the highest version correctly. However, I don't know how to get the second highest version and I don't know how to go about removing the rest of the versions from the directory.

I think your approach is too complicated and esoteric with the use of BASH_REMATCH. Here goes my solution:

versions_to_keep=(
    $(find -maxdepth 1 ! -path . -type d -printf "%f\n"\
        | sort -n -t '_' -k 2 | sort -n -m -t '.' -k 2,3 | tail -n 2)
)
highest_version=${versions_to_keep[-1]}
second_highest_version=${versions_to_keep[-2]}
echo "The highest version is: ${highest_version:?}"
echo "The second highest version is: ${second_highest_version:?}"
find -maxdepth 1 ! -path . -type d ! -name "${highest_version}"\
    ! -name "${second_highest_version}" -exec rm -rf {} +

Explanation:

find -maxdepth 1 ! -path . -type d -printf "%f\n": find and print the basename of directories at most one level below and excluding the current directory .
sort -n -t '_' -k 2: sort in numerical order by field 2, fields delimited by _
sort -n -m -t '.' -k 2,3: sort in numerical order by fields 2 to 3, fields delimited by ., but take care to merge already sorted results
tail -n 2: output the last 2 lines
${highest_version:?} and ${second_highest_version:?}: if either of those variables is null or unset, print an error message and abort the script
find -maxdepth 1 ! -path . -type d ! -name "${highest_version}" ! -name "${second_highest_version}" -exec rm -rf {} +: find directories at most one level below the current directory whose basename patterns match neither ${highest_version} nor ${second_highest_version} and delete them

Actually, if you can guarantee the directory names are of the format XXX_a.b.c.d.TEST, you can combine the two sort into one:
sort -n -t '.' -k 1.5,3, which starts the sort at character position 5 in field 1 and ends the sort at field 3. If you only want to delete empty directories, you can replace -exec rm -rf {} + with -delete.


Hiding multiple elements' parent container (by child element id) Summary:

I need to hide several (pre-determined) divs based on a child element's ID.

Caviats:

I can't alter the HTML directly in any way (by modifying the literal .html file).  The IDs of the child elements for which the parents are to be hidden are known and not likely to change often (or even at all).

I can't add CSS that will be available to the HTML page.

Here is the solution that I came up with

JavaScript

var toHide = $("#item2, #item3, #item6");
toHide.parent().hide();

HTML

item 1

related item stuff

item 2

related item stuff

item 3

related item stuff

item 4

related item stuff

item 5

related item stuff

item 6

related item stuff

item 7

related item stuff

Is there a better way to select these multiple elements than by using $("#item2, #item3, #item6")?

Are there any options I'm missing?

That is pretty close to the optimal way of doing it.

Personally I would go for

//Hide items
$("#item2, #item3, #item6").parent().hide();


Control the input of the user according to a sample string and get some info from it I made this Python code that prints a sample string and ensures that the input of the user agrees with the sample, and then do an action with this inputted string, in this case a multiplication table.

I've made this code to practice some Python concepts, and I do not know if I did everything correctly and "in a pythonic way". I'd like to know if there's anything in this code that can be improved and things I could have done otherwise.

import re
from time import sleep

class StringController(object):
    def __init__(self, *args, **kwargs):
        self.regexp = re.compile('(?i)^show me the multiplication table of [0-9]* from [0-9]* to [0-9]*$')

    def __enter__(self, *args, **kwargs):
        print('Example string: show me the multiplication table of 10 from 5 to 20')
        self.inputedstring = input('Input: ')
        print('-'*50)
        sleep(1)
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        if exc_type:
            print('{}: {}'.format(exc_type.__name__, 'You must use the provided string'))
        else:
            print('-'*50)
        del self.inputedstring

if __name__ == '__main__':
    myobject = StringController()
    while True:
        try:
            with myobject as strcon:
                multiplication_table = list(range(int(myobject.inputedstring.split(' ')[8]), int(myobject.inputedstring.split(' ')[10])+1))
                results = list(map(lambda x : x*int(myobject.inputedstring.split(' ')[6]), multiplication_table))

                if myobject.regexp.match(myobject.inputedstring):
                    print('\n'.join(str(results) for results in results))
                else:
                    raise UserWarning('You must use the provided string')

        except (ValueError, IndexError) as WrongInput:
            print("\n")
            del WrongInput

        finally:
            del strcon

Output:

Bug

The regex is flawed, consider this:

>>> print(re.match(pat, 'show me the multiplication table of  from  to '))
>>> <re.Match object; span=(0, 46), match='show me the multiplication table of  from  to '>

That is because [0-9]* matches the digit zero, or more times

I suggest to change them with + which will match the digit 1, or more times

pattern = re.compile('(?i)^show me the multiplication table of \d+ from \d+ to \d+$')

Codestyle

StringController() seems overly complex

Basically the same can be achieved with re.match(pattern, input_string)

It adds a layer of complexity what is not needed, I would just remove the entire class
The extraction of numbers can be greatly simplified

You could use re.findall(r'(\d+)', input) to find the digits in a neater way

And you could map(int, iterable) to convert them to integers

Alternative

INPUT_REGEX = re.compile('(?i)^show me the multiplication table of \d+ from \d+ to \d+$')

def make_table(user_input):
    step, min_range, max_range = map(int, re.findall('(\d+)', user_input))
    return "\n".join(str(step * t) for t in range(min_range, max_range))

def main():
    while True:
        user_input = input('Example string: show me the multiplication table of 8 from 3 to 8\nInput: ')
        print('-'*50)
        if user_input.lower() in ('q', 'quit', 'exit'):
            exit()
        elif re.match(INPUT_REGEX, user_input):
            print(make_table(user_input))
        else:
            print('Use the provided string')

if __name__ == '__main__':
    main()

Note I added a quit, might be nice for your user to let them exit in a clean way ;)


Select items between two dates that are available I wrote this MySQL query, but I feel it could be better. I had to get all clients between today and 14 days' time, that have at least one day "free". I have a SQL Fiddle, and there you can see how my schema looks, how my query looks and what it returns.

SELECT cl.id as clientId, a.id as attendantId, ca.id as caringId,
ca.startDate, ca.endDate, a.name as attendantName,
IFNULL(
  SUM(eca.effectiveDays),
  0
) AS `sumDays`
FROM client cl
LEFT JOIN
(
  SELECT ca.id,
  GREATEST(ca.startDate, CURDATE()) as `effectiveStartDate`,
  LEAST(ca.endDate, DATE_ADD(CURDATE(), INTERVAL 14 DAY)) as `effectiveEndDate`,
    DATEDIFF(
      DATE_ADD(LEAST(ca.endDate, DATE_ADD(CURDATE(), INTERVAL 14 DAY)), INTERVAL 1 DAY),
      GREATEST(ca.startDate, CURDATE())
  ) as `effectiveDays`,
  ca.clientId
  FROM caring ca
  WHERE ca.startDate <= DATE_ADD(CURDATE(), INTERVAL 14 DAY)
  AND ca.endDate >= CURDATE()
) eca
ON eca.clientId = cl.id
LEFT JOIN caring ca ON ca.clientId = cl.id
LEFT JOIN attendant a ON a.id = ca.attendantId
GROUP BY cl.id HAVING sumDays < DATEDIFF(DATE_ADD(DATE_ADD(CURDATE(), INTERVAL 14 DAY), INTERVAL 1 DAY), CURDATE())

Here you can see an example of my data:

In the marked period, I have to return John Doe and Steve Smith, because  John Doe has 3 days free in that time span, and Steve Smith has all days free.

How could I improve this query, or is there any better way of getting desired data?

I will say that you are probably always going to get into really convoluted queries like these until you begin to treat your caring data at its most granular level (a day in this case), rather than as a range of days, and  introduce the concept of a calendar/date "dimension" table that can be joined against to give all dates in a given time range, as is frequently used in scheduling applications like this.

Take for example a case where tables look like:

CREATE TABLE calendar
    (`date_idx` date NOT NULL, PRIMARY KEY `date_idx`)

/* Add all dates you might be interested in from past dates well into the future */
INSERT INTO calendar (`date_idx`) VALUES
('2017-01-01'), ('2017-01-02'), ...

/* This table now only has a single date field */
CREATE TABLE caring
    (`id` int, `careDate` date, `clientId` int, `attendantId` int);

Using these two tables, you can use the following derived table in any query to join against (or use as subselect) to compare dates in caring against the full list of available dates on calendar.

SELECT
    calendar.date_idx AS dateIdx,
    caring.id AS caringId,
    caring.clientId AS clientId,
    caring.attendantId AS attendantId,
FROM calendar
LEFT JOIN caring
    ON calendar.date_idx = caring.caringDate
WHERE
    caring.caringDate IS NULL
    AND dateIdx BETWEEN ? AND ? /* your dates of interest */
ORDER BY dateIdx ASC

This means it would be REALLY simple to extend this to your use case.

SELECT
    client.id AS clientId,
    COUNT(openDates.dateIdx) AS openDateCount
FROM client
LEFT JOIN (
    /* select shown above */
) as openDates
    ON client.id = dateRange.clientId
GROUP BY clientId
HAVING openDateCount > 1

Note that the JOIN and WHERE conditions on the derived table above can easily be changed to get different comparisons of dates in caring to all available dates in calendar.  The real "magic" is really just in having a dimension table which represents the full compliment of available "slots" within the schedule that you can easily join against. You often see similar date/time based dimension table representing different levels of granularity in scheduling applications such as this as well as in data warehouse/reporting applications.

If you search for "time dimension table script" or similar, you will find handy scripts for generating different aggregations/granularity for time into tables that can be used in your applications.

Now this may lead you to the question of how to handle sequences of dates (e.g. a customer orders a 10 day block of services).  This may be a case where you consciously decide to de-normalize your database.

So perhaps you have something like:

CREATE TABLE careOrders
    (`id` int, `careStartDate` date, `careEndDate` date, `clientId` int, `orderDate` date);

Which would be used with:

CREATE TABLE caring
    (`id` int, `careDate` date, `careOrderId` int, `attendantId` int);

Every time you create an order you enter it in the careOrders table and then build the associated items in caring to fill out time slots, leaving a reference in the caring table to the order that populated the item into the table. This is de-normalization of data as you are storing a calculated result of the order start and end dates in caring table which could be seen as redundant to the data in the careOrder table, but in fact is there to allow you to optimize queries against this data.

You perhaps even leave the clientId in the caring table (another conscious decision to break normalization) in order to optimize queries like the one in question here (i.e. to not have to include the careOrders table in the query).

Note how I used date fields instead of varchar in my examples.  You should be using the appropriate data type here.

I noted in your fiddle that none of your tables had appropriate primary keys or indexes.  I don't know if this was simply for benefit of given example (it shouldn't be on Code Review where we want to see actual production code), but this certainly should not be the case in your production environment.  You need appropriate indexes on your tables to allow the queries to be performant.

Consider using meaningful (full) names in your queries.  I know there are a lot of examples out there of people aliasing tables or derived tables with one or two character aliases, but what do you gain in doing this?  You avoid typing a few characters (that most good IDEs could autocomplete anyway)?  So what?  Bias your code (including SQL code) for readability.

Even as I read your SELECT statement, I find myself jumping around in the rest of the complex query trying to find out what a, ca, eca, etc. mean.

Same thing with the fields in your SELECT. They seem to be arbitrarily broken up across lines. Why? It just looks more cluttered to the eye.  Is there really any problem in having a query that extends more vertically down the page (the way the eye naturally wants to move)?

Even if I were to combine my example above in to one full query like:

SELECT
    client.id AS clientId,
    COUNT(openDates.dateIdx) AS openDateCount
FROM client
LEFT JOIN (
    SELECT
        calendar.date_idx AS dateIdx,
        caring.id AS caringId,
        caring.clientId AS clientId,
        caring.attendantId AS attendantId,
    FROM calendar
    LEFT JOIN caring
        ON calendar.date_idx = caring.caringDate
    WHERE
        caring.caringDate IS NULL
        AND dateIdx BETWEEN ? AND ?
    ORDER BY dateIdx ASC
) as openDates
    ON client.id = dateRange.clientId
GROUP BY clientId
HAVING openDateCount > 1

Is that not much easier to read (even without use of aliases)?

Be careful using camelCasing in MySQL schema object names. MySQL tables could suffer from OS-level differences in case sensitivity for their names, while columns, indexes, column aliases, etc. are not case sensitive at all in MySQL. For this reason, many who develop regularly with MySQL prefer snake_case for all schema objects avoid potential problems altogether.

AS is MySQL reserved word and should be strongly considered for UPPERCASE usage in your queries to help differentiate it from you database or value references.  I think you do a good job of uppercasing key and reserved words in all other cases here. as is treated inconsistently.

Why alias some fields in your select clause and not others?  I would think you would want to be consistent here. My guess is since startDate and endDate are not aliased here, and are really not relevant to the result, perhaps they should not be in select at all?