How much time is left? rags-to-riches of Convert seconds to hours, minutes and seconds.

In that question, BigDecimal was chosen to be the model class, which doesn't seem like a right fit for temporal amounts.

Sure, doing arithmetic should be the simpler and faster way, but I thought of using the Duration class to make this more 'API-driven'.

DurationFormatter

public final class DurationFormatter {

    private DurationFormatter() {
        // empty
    }

    public static String displayReadable(long seconds) {
        return displayReadable(Duration.ofSeconds(seconds));
    }

    public static String displayReadable(Duration duration) {
        if (Objects.requireNonNull(duration).isNegative()) {
            throw new IllegalArgumentException("seconds must not be negative.");
        }
        long hours = duration.toHours();
        long minutes = duration.minusHours(hours).toMinutes();
        long leftover = duration.minusHours(hours).minusMinutes(minutes).getSeconds();
        return replaceLast(
                Stream.of(append(hours, ChronoUnit.HOURS),
                            append(minutes, ChronoUnit.MINUTES),
                            append(leftover, ChronoUnit.SECONDS))
                        .filter(s -> !s.startsWith("0"))
                        .collect(Collectors.joining(", ")), ", ", " and");
    }

    private static String append(long value, ChronoUnit unit) {
        return String.format("%d %s", Long.valueOf(value),
                value == 1 ? singular(unit) : unit).toLowerCase();
    }

    private static String singular(ChronoUnit unit) {
        String toString = unit.toString();
        return toString.substring(0, toString.length() - 1);
    }

    private static String replaceLast(String input, String target, String replacement) {
        int lastIndex = input.lastIndexOf(target);
        return lastIndex == -1 ? input : input.substring(0, lastIndex) + replacement
                + input.substring(lastIndex + 1);
    }
}

DurationFormatterTest

public class DurationFormatterTest {

    enum TestCase {
        ZERO(0, ""),
        ONE_SEC(1, "1 second"),
        TWO_SECS(2, "2 seconds"),
        ONE_MIN(60, "1 minute"),
        TWO_MINS(120, "2 minutes"),
        ONE_MIN_ONE_SEC(61, "1 minute and 1 second"),
        ONE_HR(3600, "1 hour"),
        TWO_HRS(7200, "2 hours"),
        ONE_HR_ONE_SEC(3601, "1 hour and 1 second"),
        ONE_HR_ONE_MIN(3660, "1 hour and 1 minute"),
        ONE_HR_ONE_MIN_ONE_SEC(3661, "1 hour, 1 minute and 1 second"),
        MAX(Long.MAX_VALUE, "2562047788015215 hours, 30 minutes and 7 seconds");

        private final long seconds;
        private final String expected;

        private TestCase(long seconds, String expected) {
            this.seconds = seconds;
            this.expected = expected;
        }

        void verify() {
            assertThat(DurationFormatter.displayReadable(seconds), equalTo(expected));
        }

        @Override
        public String toString() {
            return String.format("%d seconds => \"%s\"", Long.valueOf(seconds),
                    expected);
        }
    }

    @DataProvider(name = "test-cases")
    public Iterator<Object[]> getTestCases() {
        return EnumSet.allOf(TestCase.class).stream().map(v -> new Object[] { v })
                .iterator();
    }

    @Test(dataProvider = "test-cases")
    public void testCases(TestCase testCase) {
        testCase.verify();
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void testNegativeValue() {
        try {
            DurationFormatter.displayReadable(-1);
        } catch (IllegalArgumentException e) {
            assertThat(e.getMessage(), equalTo("seconds must not be negative."));
            throw e;
        }
    }

    @Test(expectedExceptions = NullPointerException.class)
    public void testNullValue() {
        DurationFormatter.displayReadable(null);
    }
}

After sitting on this for... 8 hours, I realized singular() can, and probably should, use the replaceLast() method, to encourage code re-use at a very small expense of efficiency (which isn't the goal here anyways):

private static String singular(ChronoUnit unit) {
    return replaceLast(unit.toString(), "s", "");
}


Is there an easier way to find the index of the first letter in a string? I have a function that returns the index of the first alphabet letter found in a string, or -1 if no letter is found.  It seems like there should be a way to do this with extension methods instead of looping, but I can't think of one:

int findFirstLetter(string str)
{
 for(int ctr=0;ctr<str.Length;ctr++)
 {
  if (Char.IsLetter(str[ctr]))
  {
   return ctr;
  }
 }
 return -1;
}

I have no issue with the loop. It is about as fast as it can be. I would recommend renaming the method to match the other IndexOf methods.... something like IndexOfAlphaChar.

There is no reason to worry about this code. Move on to bigger problems.


Simulating a polygon This code simulates a polygon (say a triangle) with a person at each vertex, every person is looking at the next adjacent vertex/person. They all start moving towards each other with a constant velocity "trying to chase other". I could not understand this conceptually so I created an applet in Java to help me understand it in physics.

package test;

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.GeneralPath;
import java.util.ArrayList;

public class ChasePolygon extends Applet {

    public void paint(Graphics g) {
        final int radius = 200;
        final float velocity = 0.001f;
        final int sides = 6;
        final float theta = (float) (2 * Math.PI / sides);
        final int centerX = 200, centerY = 200;
        ArrayList<Float[]> vertices_loc = new ArrayList<Float[]>(sides);
        for (int i = 0; i < sides; i++) {
            final float pX = (float) (centerX + radius * Math.cos(i * theta));
            final float pY = (float) (centerY + radius * Math.sin(i * theta));
            vertices_loc.add(new Float[] { pX, pY });
        }
        Graphics2D g2 = (Graphics2D) g;
        GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
        polyline.moveTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
        for (int i = 0; i < sides; i++) {
            polyline.lineTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
        }
        polyline.lineTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
        g2.draw(polyline);
        int delta_T = (int) Math
                .floor((radius * Math.sin(theta / 2) / velocity));
        ArrayList<GeneralPath> g_p = new ArrayList<GeneralPath>(sides);
        for (int i = 0; i < sides; i++) {
            g_p.add(new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T));
            g_p.get(i).moveTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
        }
        for (int i = 0; i < delta_T; i++) {
            for (int j = 0; j < sides; j++) {
                float dX = 0, dY = 0;
                if (j != sides - 1) {
                    dX = vertices_loc.get(j + 1)[0] - vertices_loc.get(j)[0];
                    dY = vertices_loc.get(j + 1)[1] - vertices_loc.get(j)[1];
                } else {
                    dX = vertices_loc.get(0)[0] - vertices_loc.get(j)[0];
                    dY = vertices_loc.get(0)[1] - vertices_loc.get(j)[1];
                }
                vertices_loc.get(j)[0] += dX * velocity;
                vertices_loc.get(j)[1] += dY * velocity;
                g_p.get(j).lineTo(vertices_loc.get(j)[0],
                        vertices_loc.get(j)[1]);
            }
        }
        for (int i = 0; i < sides; i++) {
            g2.draw(g_p.get(i));
        }

    }
}

Here is the sample output:

\$\hskip1.25in\$

Improvements could be (not necessarily these but any you consider):

Better way/s to do certain things, specific to programming only (not to my algorithm).
My first attempt with Java graphics/Swing etc. so any advice/hints etc.
Performance. Yes, it's slow; try running it.
It restarts when I change applet size.
Fix applet size, etc. as you wish.

I ran through your code, and agree that the output is pretty. I like the idea, and it is a good use-case for a number of exercises. Your code suffers though from a few issues that are awkward.

First up, you have just one real method. Why? You should isolate discrete logic elements in to their own methods. That's the most apparent issue, but we'll get there, but other things get in the way first.

Constants

While we are isolating things, why do you redeclare constant values each time you call paint? Let's take the first chunk of your code:

public class ChasePolygon extends Applet {

    public void paint(Graphics g) {
        final int radius = 200;
        final float velocity = 0.001f;
        final int sides = 6;
        final float theta = (float) (2 * Math.PI / sides);
        final int centerX = 200, centerY = 200;

That should be:

public class ChasePolygon extends Applet {

    private final int radius = 200;
    private final float velocity = 0.001f;
    private final int sides = 6;
    private final float theta = (float) (2 * Math.PI / sides);
    private final int centerX = 200;
    private final int centerY = 200;

    public void paint(Graphics g) {

That's making a bunch of your constants... constants.

Floats

Floats are an awkward value type in Java. You have to keep casting them as floats because the 'natural' floating-point number is a double. I recommend tossing floats out unless you are space-constrained. You are not space constrained, so all your floats should just be doubles. This makes your code a bunch easier to read... for example, lines like:

final float pX = (float) (centerX + radius * Math.cos(i * theta));

become just:

final double pX = centerX + radius * Math.cos(i * theta);

vertices_loc

This ArrayList is a decent concept, but, your code would become a lot easier, and all the 1's and 0's that are scattered though your code would be redundant if you just created two double[] arrays. Consider:

double[] verticesX = new double[sides];
double[] verticesY = new double[sides];

Now you lose your [0] and [1] subscripts for X and Y instead. It is shorter, and even has more meaning. Double-win.

Function Extraction

Right, now it is time for function extraction. First up, extract the polygon draw. We have a trick we can do there too...

Your current polygon draw code is:

GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
polyline.moveTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
for (int i = 0; i < sides; i++) {
    polyline.lineTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
}
polyline.lineTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
g2.draw(polyline);

Note how you start at (moveTo) vertix 0, and your first line draws to (lineTo) vertix 0? You essentially start with a 0-length line, and then you have to fix that with a post-loop line-to vertix 0.

A neater solution would be to just start at the last vertex, and then go from there to the first.... But, put that in a function too. Using my suggestion for the verticesX and verticesY, it becomes (note the sides - 1 moveTo):

private void paintPolygon(Graphics2D g2, double[] verticesX, double[] verticesY) {
    GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
    polyline.moveTo(verticesX[sides - 1], verticesY[sides - 1]);
    for (int i = 0; i < sides; i++) {
        polyline.lineTo(verticesX[i], verticesY[i]);
    }
    g2.draw(polyline);
}

Spirals

The final functional extraction comes from the spiral paths. These should be extracted. Again, using an actual array makes it easier than using an ArrayList. Additionally, the final print loop would be better as a for-each loop. That final method becomes:

private void paintCurves(Graphics2D g2, double[] verticesX, double[] verticesY) {
    GeneralPath[] g_p = new GeneralPath[sides];
    for (int i = 0; i < sides; i++) {
        g_p[i] = new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T);
        g_p[i].moveTo(verticesX[i], verticesY[i]);
    }
    for (int i = 0; i < delta_T; i++) {

        for (int j = 0; j < sides; j++) {
            double dX = verticesX[(j + 1) % sides] - verticesX[j];
            double dY = verticesY[(j + 1) % sides] - verticesY[j];
            verticesX[j] += dX * velocity;
            verticesY[j] += dY * velocity;
            g_p[j].lineTo(verticesX[j], verticesY[j]);
        }
    }
    for (GeneralPath gp : g_p) {
        g2.draw(gp);
    }
}

Constant vertices.

Finally, after simplifying things, it became apparent that the initial vertices are constant, and you then overwrite them as you spiral in. This makes it simpler to calculate the vertices once, use those for the polygon, and then use a copy for the spirals. The final paint method is just:

public void paint(Graphics g) {

    Graphics2D g2 = (Graphics2D) g;
    paintPolygon(g2);
    paintCurves(g2, Arrays.copyOf(verticesX, sides), Arrays.copyOf(verticesY, sides));

}

Clean version

All told, I would go with the following:

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.GeneralPath;
import java.util.ArrayList;
import java.util.Arrays;

public class ChasePolygon extends Applet {

    private final int radius = 200;
    private final double velocity = 0.001;
    private final int sides = 7;
    private final double theta = 2.0 * Math.PI / sides;
    private final int centerX = 200;
    private final int centerY = 200;
    private final int delta_T;
    private final double[] verticesX = new double[sides];
    private final double[] verticesY = new double[sides];

    public ChasePolygon() {
        delta_T = (int) Math.floor(radius * Math.sin(theta / 2) / velocity);
        for (int i = 0; i < sides; i++) {
            verticesX[i] = centerX + radius * Math.cos(i * theta);
            verticesY[i] = centerY + radius * Math.sin(i * theta);
        }

    }

    public void paint(Graphics g) {

        Graphics2D g2 = (Graphics2D) g;
        paintPolygon(g2);
        paintCurves(g2, Arrays.copyOf(verticesX, sides), Arrays.copyOf(verticesY, sides));

    }

    private void paintCurves(Graphics2D g2, double[] vertX, double[] vertY) {
        GeneralPath[] g_p = new GeneralPath[sides];
        for (int i = 0; i < sides; i++) {
            g_p[i] = new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T);
            g_p[i].moveTo(vertX[i], vertY[i]);
        }
        for (int i = 0; i < delta_T; i++) {

            for (int j = 0; j < sides; j++) {
                double dX = vertX[(j + 1) % sides] - vertX[j];
                double dY = vertY[(j + 1) % sides] - vertY[j];
                vertX[j] += dX * velocity;
                vertY[j] += dY * velocity;
                g_p[j].lineTo(vertX[j], vertY[j]);
            }
        }
        for (GeneralPath gp : g_p) {
            g2.draw(gp);
        }
    }

    private void paintPolygon(Graphics2D g2) {
        GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
        polyline.moveTo(verticesX[sides - 1], verticesY[sides - 1]);
        for (int i = 0; i < sides; i++) {
            polyline.lineTo(verticesX[i], verticesY[i]);
        }
        g2.draw(polyline);
    }

}


Class for Stripe transactions I just wonder whether it is a common approach to hit the third-party API and fetch values within the initialize method?

Ex.

    class BalanceTransaction::Update
      include Service

      attr_reader :offset, :transactions

      def initialize(offset)
        @offset = offset
        @transactions = fetch_transactions
      end

      def call
        transactions.auto_paging_each do |txn|
          type = txn[:type]
          source = txn[:source]

          case type
            when 'charge', 'adjustment' # NOTE: adjustment is something
              invoice = find_invoice(source)
              ac_id = invoice.ac.id
              update_attrs(id: invoice.id, account_id: ac_id, type:'invoice', attrs: txn)
            when 'refund'
              refund = find_refund(source)
              ac_id = refund.invoice.account.id
              update_attrs(id: refund.id, account_id: ac_id, type:'refund', attrs: txn)
          end
        end
      end

   private
      def external_card_balance
        Stripe::BalanceTransaction
      end

      def fetch_transactions
       external_card_balance.all
      end

Since it seems you only need the transactions value once, you might as well just make an attribute reader that does the work:

  def transactions
    external_card_balance.all
  end

No real need to break in down into "fetch and store" and "read from store and iterate", when you can just say "fetch and iterate".

If you're going to be using the transactions collection multiple times, you can either memoize it:

  def transactions
    @transactions ||= external_card_balance.all
  end

Or you can simply pass the transactions as a constructor argument. The latter moves the responsibility for fetching transactions to the user of the class, rather than the class itself, which may make more sense - or it may not. Your call.


Censor the middle of words of length N from a text file LENGTHS_TO_CENSOR = {4, 5}
CENSOR_CHAR       = '*'
CENSOR_EXT        = "-censored"

def replace_inner(word, char):
    if len(word) < 3:
        return word
    return word[0] + char * len(word[1:-1]) + word[-1]

def create_censor_file(filename):
    output_file = open(filename + CENSOR_EXT, "w+")
    with open(filename) as source:
        for line in source:
            idx = 0

            while idx < len(line):

                # If the character isn't a letter, write it to the output file.
                if not line[idx].isalpha():
                    output_file.write(line[idx])
                    idx += 1

                else:
                    word = ""

                    while idx < len(line) and line[idx].isalpha():
                        word += line[idx]
                        idx += 1

                    if len(word) in LENGTHS_TO_CENSOR:
                        word = replace_inner(word, CENSOR_CHAR)

                    output_file.write(word)

    output_file.close()

def main():
    filename = input("File to be censored: ")
    create_censor_file(filename)

if __name__ == "__main__":
    main()

I was assigned a task to censor words that are length n in a file. This file can potentially contain punctuation and numbers.
I originally tackled the problem by splitting the line into a list of words (using .split(' ')) and checking the length to determine if the program should censor the word or not. This failed for inputs such as:

does not work.for.this.input
or.this

The output file must be exactly like the input but with words of length in LENGTHS_TO_CENSOR censored with CENSOR_CHAR.
I decided to abandon trying to make it Pythonic and ended up with this result. I want to know if there is a way to take this method and make it more Pythonic.

create_censor_file should really be called create_censored_file.
I'd rename source to source_file for consistency and clarity.
You should use with for both files.
Why not use just w instead of w+?
This is probably one of the few things that regexes are actually useful for. You can just use re.sub(r'(?<=\b\w)\w{' + ','.join(map(lambda x: str(x-2), LENGTHS_TO_CENSOR)) + '}(?=\w\b)', lambda match: CENCOR_CHAR * len(match.group(0)), source.read())

A couple other things:

Good job with the main function and if __name__ == '__main__' check!
I have not yet tested this code.

Result:

LENGTHS_TO_CENSOR = {4, 5}
CENSOR_CHAR       = '*'
CENSOR_EXT        = "-censored"

def create_censor_file(filename):
    with open(filename + CENSOR_EXT, "w") as output_file, open(filename) as source_file:
        output_file.write(
            re.sub(
                r'(?<=\b\w)\w{'
                    + ','.join(map(lambda x: str(x-2), LENGTHS_TO_CENSOR))
                    + '}(?=\w\b)',
                lambda match: CENSOR_CHAR * len(match.group(0)),
                source_file.read()))

def main():
    filename = input("File to be censored: ")
    create_censor_file(filename)

if __name__ == "__main__":
    main()


Invocable callout(Post) from processbuilder I am relatively new in making (invocable)callouts. My main concern is if I should be using smaller parts for example for the body and using a query only once in this case. Any suggestions would be appreciated.

Class:

public class or_service {
    @InvocableMethod
    public static void InvocePostProperty(List<ID> pIds) {
        for (Id pId : pIds) {
            PostProperty(pId);
        }
    }
    @future(callout=true)
    public static void PostProperty(Id pId) {
        List<Orbirental_Service__c> os = null;
        List<Property__c> prop = null;
        Http http = new Http();
        HttpRequest request = new HttpRequest();
        try {
            os = [SELECT Url__c, ApiKey__c, agencyUid__c FROM Orbirental_Service__c LIMIT 1];
            prop = [SELECT id,  Name__c, BaseGuests__c, maximumGuests__c, baseDailyRate__c, city__c, state__c, acceptInstantBook__c, isActive__c ,uid__c
                    FROM Property__c  WHERE id =:pId LIMIT 1];
        }catch(QueryException ex) {
            os = null;
            prop = null;
        }
        request.setEndpoint(os[0].Url__c +'v1/properties/');
        request.setMethod('POST');
        request.setHeader('Content-Type', 'application/json');
        request.setHeader('X-ORBIRENTAL-APIKEY', os[0].ApiKey__c);
        // Set the body as a JSON object
        request.setBody('{"type": "HOUSE"'+
                        ',"name":' + prop[0].Name__c +
                        ',"agencyUid":' + os[0].agencyUid__c +
                        ',"baseGuests":' + prop[0].BaseGuests__c +
                        ',"maximumGuests":' + prop[0].maximumGuests__c +
                        ',"baseDailyRate":' + prop[0].baseDailyRate__c +
                        ',"city":' + prop[0].city__c +
                        ',"state":'+ prop[0].state__c +
                        ',"acceptInstantBook":'+ prop[0].acceptInstantBook__c +
                        ',"isActive":'+ prop[0].isActive__c+
                        '}');
        HttpResponse response = http.send(request);
        // Parse the JSON response
        if (response.getStatusCode() != 200) {
            System.debug('The status code returned was not expected: ' +
                         response.getStatusCode() + ' ' + response.getStatus());
        } else {
            System.debug(response.getBody());
            updateProperty(response.getBody(),pId);
        }
    }

    public static void updateProperty(string jsonString, Id pId) {
        List<Property__c> prop = null;
        or_propertyJSON propClass = or_propertyJSON.parse(jsonString);
        System.debug('myClass.uid '+ propClass.uid);
        try {
            prop = [SELECT id,  Name__c, BaseGuests__c, maximumGuests__c, baseDailyRate__c, city__c, state__c, acceptInstantBook__c, isActive__c ,uid__c
                    FROM Property__c  WHERE id =:pId LIMIT 1];
        }catch(QueryException ex) {
            prop = null;
        }
        prop[0].uid__c = propClass.uid;
        update prop;
    }
}

From a practical viewpoint, I think the methods that you have right now are splitting the responsibilities of your overall code fairly appropriately. You could further split things up, but the question to ask is what purpose would this serve?

The more methods and classes you break things into, the more you need to jump around in your class(es) to learn (if you're new to the codebase)/remember (if it has been long enough since you last touched the code) how things work.

Instead of focusing on that, there are more pressing issues:

Exceptions as flow control

This is generally viewed as an antipattern (i.e. something to avoid). The main purpose of exceptions is to (more) gracefully handle error conditions where continued processing is impossible.
In this particular case, running into a System.QueryException should be impossible. In the case where either of your queries returns 0 rows, you can't really continue processing. You'd run into a System.ListException when you try to access  os[0] or prop[0]

Non-bulkified @future call

You're calling your @future method one Id at a time. You can only make 50 @future calls in a given transaction, so if your ProcessBuilder tries to work on 51 records, you're liable to run into this governor limit.
The better option here would be to pass the entire list you accept as an argument to your InvocePostProperty() method to your @future method as well
Building on the previous suggestion, if possible, you should pass more than one event (I'm assuming it's an event service you're using, based on the language) per callout

No notification when async code fails

Asynchronous code, at least for me, has a pretty bad history of not printing debugs to logs. The fact that code is being run async also means that there's no direct way to alert you if the async portion of the code fails.
In my org, using async code means that we need to do our best to catch all possible errors, and create a Case or send an email to an appropriate person to let them know that a failure has occurred

My Suggestions:

I think that Queueable apex is probably more appropriate in your case. Unlike @future methods, queueable apex can be chained ad infinitum
Add explicit error notification to your async method(s)
Attempt to make more than one callout per async transaction. You can have 100 callouts in a single transaction (as long as your callouts in a given transaction don't collectively exceed 120 seconds of execution time)
Make use of the Limits class to determine whether or not you can make another callout in this transaction.
Don't forget to store the records you want to update in a list, and perform DML after you finish all of your callouts for a given transaction
Instead of trying to catch (and swallow) a System.QueryException, add a custom exception inner class to your or_service class. If you detect 0 rows returned by either of your queries, throw a new instance of this exception (and then do explicit notification)


Refactor code to find four co-linear points I am relatively new to Java, but have been reading up on how to improve the quality of my code.

I am writing a system where I take in a series of points from a file with x and y co-ordinates and by checking slopes relative to each other, am calculating and drawing a line if there is four or more co-linear points on the line. The code works and is doing what is required, but I would like to tidy up this method by reducing the if statements, but am not sure on how to, or if it is even necessary. The parameters being passed in is an array of doubles, which are slopes relative to an origin.  I am iterating this array to look for 3 or more equal slopes in a row which would give a line and adding the corresponding points and any further equal points to a set.  Once a line has been found, the rest of the slopes in array are checked in similar fashion. Any advice would be appreciated.

I think it's messy, but not sure how to rectify it.

public static void checkForLines(Double[] arrayOfSlopes, Point[] tempArray)
{
    int count = 0; // Keeps track of number of duplicates
    int i = 1;

    while (i < arrayOfSlopes.length - 1)
    {
        Double first = arrayOfSlopes[i];
        Double next = arrayOfSlopes[i + 1];

        if (first.equals(next))
            // Compares two consecutive indexes in array for equality
        {
            count = count + 1; // If match found increase the count

            if (count == 2)
                // At this point a line has been found
            {
                SortedSet<Point> line = new TreeSet<Point>();
                //Create a set to store points from this line
                line.add(tempArray[0]);
                line.add(tempArray[i - 1]);
                line.add(tempArray[i]);
                line.add(tempArray[i + 1]);
                //Store four found points

                int j = i + 1;

                while (j + 1 < arrayOfSlopes.length - 1 && (arrayOfSlopes[j].equals(arrayOfSlopes[j + 1])))
                {
                    line.add(tempArray[j + 1]);
                    //Store any other further duplicates if exist
                    j++;
                }

                if (arrayOfSlopes[j].isInfinite() && arrayOfSlopes[arrayOfSlopes.length - 1].isInfinite() ||
                    arrayOfSlopes[arrayOfSlopes.length - 1].equals(first))
                {
                    line.add(tempArray[tempArray.length - 1]);
                    //Check the last element for equality
                }

                if (lines.isEmpty())
                {
                    lines.add(line);
                }

                else
                {
                    lines.add(0, line);
                    ;
                    weedDuplicates(line);
                }
                i = j + 1; //Continue iteration from this point
            }

            if (count < 2)
            {
                i++;
            }
        }

        else if (first.equals(next) == false)
        {
            count = 0;
            i++;
        }
    }
}

The easiest way to flatten this kind of a structure is to reverse the sense of the if.  That is, where you have

if (first.equals(next)) {
    // do stuff
}

Instead write

if (!first.equals(next)) {
    continue;
}
// do stuff

This only works where you don't have an else clause (so unfortunately I chose a bad example), but it can work wonders in bringing your code closer to the left margin.

From there I would start seeing where I could extract methods, especially but not exclusively where there is duplication in the code.  The block of code beginning with

SortedSet<Point> line = new TreeSet<Point>();

looks like a good candidate for this.


How should I structure my Unit tests for minimal Asserts? I always see comments about how a Unit Test should only be testing one piece of functionality. For the most part, I definitely agree with that assessment. However, I'm working on testing a method right now that has me pondering if being laxer on this ideal would be a better thing, so I'm curious to everyone's opinions.

The method that I'm testing is one that gathers Messages created between a user-defined date range, and groups them by Date, with totals of each type of Message per day. (Request, Offer, Dialog, and Referral).

So for a range of two days (for simplicity), I'll return an IEnumerable<MessageDay>, where MessageDay is defined as such:

public class MessageDay() {
    public DateTime Date { get; set; }
    public int Requests { get; set; }
    public int Offers { get; set; }
    public int Dialogs { get; set; }
    public int Referrals { get; set; }
    public int Total {
        get {
            return this.Requests + this.Offers + this.Dialogs + this.Referrals;
        }
    }
}

I have my Mock setup for my unit test with several of each type of Message, ranging from DateTime.MinValue, DateTime.Now, and DateTime.MaxValue.

Should I create a Unit Test with the same parameters going into the method I'm testing repeatedly? (i.e. first test will test a count of the days, second test will test the total Requests for day 1, etc for each type of Message, for each date)

I'm thinking that having so many tests (up to 13 per date range with 4 different date parameter possibilities) would be ridiculous, however I would like to know the standard, accepted way to do this.

EDIT
Here is my Mock data that I have set up:

[TestInitialize()]
public void TestInitialize() {
    var mock = new Mock<IReportingRepository>();
    var now = DateTime.Now;

    mock.Setup(m => m.Messages).Returns(new Message[] {
        new Request { CreatedOn = now },
        new Request { CreatedOn = DateTime.MinValue },
        new Request { CreatedOn = DateTime.MaxValue },
        new Offer { CreatedOn = DateTime.MinValue },
        new Offer { CreatedOn = DateTime.MinValue },
        new Offer { CreatedOn = DateTime.MaxValue },
        new Dialog { CreatedOn = now },
        new Dialog { CreatedOn = DateTime.MaxValue },
        new Referral { CreatedOn = now },
        new Referral { CreatedOn = DateTime.MinValue }
    }.AsQueryable());

    controller = new ReportingController(mock.Object);
}

This would be along the lines of my "fewest number of tests":

[TestMethod]
public void CanFindAllCounts() {
    var result = (MessageResponse)controller.GetMessageAnalytics(new MessageRequest()).Data;

    Assert.AreEqual(3, result.Days.Count());

    var day = result.Days.FirstOrDefault();
    Assert.AreEqual(4, day.Total);
    Assert.AreEqual(1, day.Requests);
    Assert.AreEqual(2, day.Offers);
    Assert.AreEqual(0, day.Dialogs);
    Assert.AreEqual(1, day.Referrals);

    day = result.Days.Where(d => d.Date.Date == DateTime.Now.Date).FirstOrDefault();
    Assert.AreEqual(3, day.Total);
    Assert.AreEqual(1, day.Requests);
    Assert.AreEqual(0, day.Offers);
    Assert.AreEqual(1, day.Dialogs);
    Assert.AreEqual(1, day.Referrals);

    day = result.Days.Last();
    Assert.AreEqual(3, day.Total);
    Assert.AreEqual(1, day.Requests);
    Assert.AreEqual(1, day.Offers);
    Assert.AreEqual(1, day.Dialogs);
    Assert.AreEqual(0, day.Referrals);
}

The other extreme would of course be a test for each of those Asserts. Being that I'm using Moq, it's not a huge deal and would run quickly, but I just feel that it's overkill.

In this case, what you're really trying to do is validate three MessageDays, as the return values of one method. As you are only testing one thing (the successful return of a method), you need not worry about testing too many things. I would, however, reduce the amount of asserts to three by implementing .Equals() on MessageDay.

While it may seem like overkill, .Equals() is a virtual method on System.Object, so you do already have an implementation of it available (just not the one you need, as it is currently a reference comparison).

I can't predict whether or not you'll need it again in the future, but I don't consider that a problem when implementing the method in question, because all objects have it. The effort is small, and the chances of using it somewhere other than just testing are high.

The downside is, of course, that your test will have to declare expected values, which may mean that it becomes no shorter, and the reduced number of asserts means your test is slightly less granular. For the second point, that's probably fine, because it also frees you to redesign the class without changing the test. The length could be reduced by  comparing with values saved in the test initialization.


Reading a property on a main thread that is set on a background thread I am currently handling some setup of an object on a background thread (I am using a concurrent queue within the following initWithDocumentFileURL method):

+ (TWReaderDocument *)initWithDocumentFileURL:(NSURL *)url withLoadingCompletionBlock:(loadingCompletionBlock)completionBlock{

 __block NSError *error;
TWReaderDocument * twDoc = [[TWReaderDocument alloc] init];
twDoc.status = ReaderDocCreated;
twDoc.doc = [ReaderDocument withDocumentFilePath:[url path] withURL:url withLoadingCompletionBLock:^(NSError *_error) {

    if (_error)
    {
        _error = error;
        twDoc.status = ReaderDocReady;
    }
    else twDoc.status = ReaderDocFailed;

}];

//completionBlock(error);

return twDoc;
}

Header

@property (nonatomic, assign, readonly) TWReaderDocumentStatus status;

Implementation

- (TWReaderDocumentStatus) status {

    __block TWReaderDocumentStatus readStatus;

    dispatch_sync([ReaderDocument concurrentLoadingQueue], ^{
        readStatus = _status;
    });

    return readStatus;

}

If I would like to access the status property on the main thread do I need to take any precaution or there is nothing to take care of as I am just reading the property on the main thread?

If all you're trying to synchronize is TWReaderDocumentStatus, and if that's a simple data type (e.g. an integer or float, not an object), you can just define it as atomic, and it can be safely read and updated from multiple threads, with no custom getter required. This is one of the very few situations where atomic can be useful for synchronization (and it's remarkably efficient).

By the way, if your property has a public interface that is readonly, make sure the private class extension redefines it as readwrite so the setter is synthesized for you. And make sure that you only set it using the setter (not bypassing it and using the instance variable directly).


Is the Button, an  element with svg code and hidden link content inside, ARIA conform and well laid out? I have an a element for a button. The SVG snippet inside links to the containing SVG file at the top of the body.

   <svg viewBox="0 0 70.115 53.162" class="head__btn">
     <use xlink:href="#skmenu"></use>
   </svg>

When I checked with Codesniffer for WCAG2AA compliance it criticized:

  Anchor element found with a valid href attribute, but no link content
  has been supplied.

So I've added text inside the  element and wrapped it with a span.

Menu Toggle
   <svg viewBox="0 0 70.115 53.162" class="head__btn">
     <use xlink:href="#skmenu"></use>
   </svg>

And hid the text with the following CSS snippet. Without the span, both, the text as well as the SVG would have been hidden.

.srtext {
    position: absolute;
    top: -9999px;
    left: -9999px;
}

Question is, is the solution ARIA as well as browser compliant? Or are there ways to improve the solution (maybe without span)?

SVG has a desc and a title tag that you're meant to use to describe the SVG:

<a href="#menu" class="head__toggle" title="Toggle Menu">
    <svg viewBox="0 0 70.115 53.162" class="head__btn">
        <desc>Toggle Menu</desc>
        <use xlink:href="#skmenu"></use>
    </svg>
</a>

When you run it through CodeSniffer, you get this notice instead:

  Notice: Check that text of the link describes the purpose of the link.

That said, I don't buy that inline SVGs are the way to go, as it increases the size of the document and cannot be cached in a meaningful way.  I mean, it's not considered a good practice to place the entire contents of your CSS file in the head tag to reduce HTTP requests, why would it be good for SVGs?

You can still get most of the benefits Mr. Coyier talks about in his SVG For Everybody talk by using an external SVG embedded via the img tag, in addition to being able to cache it.

Take advantage of CSS for styling purposes (including media queries)
SVG sprite stack technique, meaning you won't have to set the viewBox information.


Is this C++ program correctly implemented in SSE? I have to implement a simple program in SSE, and I don't know if I have done it in the right way (it's my first program in SSE).

This is the C++ program:

double distance (int x1, int x2, int y1, int y2)
{
    double x = abs(x2 - x1);
    double y = abs(y2 - y1);

    double dist = sqrt(x*x + y*y);

    return dist;
}

And this is the program in SSE instructions:

__m128d vx1 _mm_load_pd(x1); // load x1 and x2
__m128d vx2 = _mm_load_pd(x2);
__m128d vdiff = _mm_sub_pd(vx2, vx1); // x2 - x1
__m128d vnegdiff = mm_sub_pd(_mm_set1_pd(0.0), vdiff); // 0.0 - diff
__m128d vabsX = _mm_max_pd(vdiff, vnegdiff);        //  abs = max(diff, - diff)

__m128d vy1 = _mm_load_pd(y1);
__m128d vy2 = _mm_load_pd(y2);
__m128d vdiff2 = _mm_sub_pd(vy2, vy1);
__m128d vnegdiff2 = mm_sub_pd(_mm_set1_pd(0.0), vdiff2);
__m128d vabsY = _mm_max_pd(vdiff2, vnegdiff2);

__m128d absX = _mm_mul_pd(vabsX,vabsX); //x*x
__m128d absY = _mm_mul_pd(vabsY,vabsY); //y*y

__m128d suma = _mm_add_pd(absX, absY); // x^2 + y^2

__m128d square = _mm_sqrt_pd(suma); // square root x^2 + y^2

return square;

Your SSE code looks like a reasonably accurate translation of your original C, but that C code doesn't immediately jump out as the best way to do things. In particular, I don't see where you gain anything by taking the absolute value before you do your multiplication. Given that your're squaring the value immediately afterwards anyway, it would appear simpler to just leave it negative if that's what it happens to be. Lacking a specific reason to do otherwise, I'd start with something simpler, like this:

double x = x1 - x2;
double y = y1 - y2;

double dist = sqrt(x * x + y * y);

Then I'd convert that to SSE instructions.

Especially if you're concerned with speed, you might want to consider using an alternative to the Pythagorean theorem to compute the hypotenuse though. I posted some C for one possibility in a previous answer.


Silly redis implementation This is a silly Redis implementation that handles only three commands: INFO, ECHO, PING.

The implementation is based on streams:

var _ = require('lodash')
var expect = require('chai').expect;
var assert = require('chai').assert;
var redis = require('redis')
var split = require('split')
var net = require('net');

var util = require('util');
var Transform = require('stream').Transform;

function makeCommand(parts) {
  var parts = _.filter(parts, (_, i) => i % 2 === 1)
  return {
    command: _.first(parts),
    args: _.drop(parts)
  }
}

function CommandParser(options) {
  if (!(this instanceof CommandParser)) {
    return new CommandParser(options)
  }

  Transform.call(this, {readableObjectMode: true})
  this.__counter = 0
  this.__parts = []
}

util.inherits(CommandParser, Transform);

CommandParser.prototype._transform = function(chunk, encoding, done) {
  var str = chunk.toString('utf-8')
  if(this.__counter == 0) {
    assert(str[0] == '*')
    this.__counter = Number(_.drop(str)) * 2
  } else {
    console.log(this.__counter, str)
    this.__parts.push(str)
    this.__counter--
  }
  if(this.__counter === 0 && !_.isEmpty(this.__parts)) {
    console.log('here')
    var command = makeCommand(this.__parts)
    console.log(command)
    this.push(command)
    this.__parts = []
  }
  done()
};

function CommandProcessor(options) {
  if (!(this instanceof CommandProcessor)) {
    return new CommandProcessor(options)
  }

  Transform.call(this, {writableObjectMode: true})
}

util.inherits(CommandProcessor, Transform);

CommandProcessor.prototype._transform = function(command, encoding, done) {
  switch (command.command) {
    case 'ping':
      done(null, '+pong\r\n')
      break;
    case 'info':
      done(null, '$9\r\nsome info\r\n')
      break;
    case 'echo':
      done(null, `\$${command.args[0].length}\r\n${command.args[0]}\r\n`)
      break;
    default:
      console.log(command)
      assert.fail()
  }
};

var TEST_PORT = 6380

var server = net.createServer(function(socket) {
  var commandParser = new CommandParser()
  var commandProcessor = new CommandProcessor()
  socket
    .pipe(split())
    .pipe(commandParser)
    .pipe(commandProcessor)
    .pipe(socket)
});

server.listen(6380, '127.0.0.1')

You can use the standard redis client to work with it. I'm mostly interested in any advice about node streams, but any other ideas are welcome too.

var parts = _.filter(parts, (_, i) => i % 2 === 1) this looks like a habit from python. I do not recommend shadowing lodash in any part of the code, it is harder to read.


Return the optimal time slot Implement a function meetingPlanner that given the availability, slotsA and slotsB, of two people and a meeting duration dur, returns the earliest time slot that works for both of them and is of duration dur. If there is no common time slot that satisfies the duration requirement, return null.

Examples:

input:  slotsA = [[10, 50], [60, 120], [140, 210]]
        slotsB = [[0, 15], [60, 70]]
        dur = 8
output: [60, 68]

input:  slotsA = [[10, 50], [60, 120], [140, 210]]
        slotsB = [[0, 15], [60, 70]]
        dur = 12
output: null # since there is no common slot whose duration is 12

This is my approach to the solution:

    import java.io.*;
    import java.util.*;
    import java.lang.*;

    class Solution {

      static int[] meetingPlanner(int[][] slotsA, int[][] slotsB, int dur) {
        // your code goes here
        int i = 0,j = 0;
        int maxStartDur,minEndDur, overlap;
        int [] optTime = new int[2];
        int [] nullArr = new int[0];
        for( i = 0; i < slotsA.length; i++)
        {
          for(j = 0; j < slotsB.length; j++ )
          {
            //Calculate the overlap between corresponding times
              maxStartDur = Math.max(slotsA[i][0],slotsB[j][0]);
              minEndDur = Math.min(slotsA[i][1], slotsB[j][1]);
              overlap = minEndDur - maxStartDur;

              if( overlap >= dur )
              {
                optTime[0] = maxStartDur;
                optTime[1] = maxStartDur + dur;
                return optTime;
              }
             /* minDur = Math.min((slotsA[i][1] - slotsA[i][0]),(slotsB[j][1] - slotsB[j][0]));
              if( minDur >= dur)
              {
                optTime[0] = slotsA[i][0];
                optTime[1] = slotsA[i][0] + dur;
              }
              else
                break;
            }
            */
          }

      }

        return nullArr;
      }
// Time complexity:  O(slotsA.length*slotsB.length)
//Space complexity: O(array of size [2])
      public static void main(String[] args) {

      }

    }

These are some of the questions regarding this code:

Can I further optimize this code to have an improved time and space complexity?
Is there a smarter approach to solve this question?
Are there better data structures available which I can use to solve this question more appropriately?

Reference

Yes, code can be optimized to O(slotsA.length + slotsB.length) asumming that slotsA and slotsB are already sorted and the time ranges in each input do not overlap (which seems to be the case from your example).
Haven´t tested code and there are probably some edge cases missing but it should give you the idea:
    int idxA, idxB = 0;
    int maxStartDur, minEndDur, overlap;

    while (idxA < slotsA.length && idxB < slotsB.length) {
        maxStartDur = Math.max(slotsA[idxA][0],slotsB[idxB][0]);
        minEndDur = Math.min(slotsA[idxA][1], slotsB[idxB][1]);
        overlap =  minEndDur - maxStartDur;

        if (overlap >= dur) {
            optTime[0] = maxStartDur;
            optTime[1] = maxStartDur + dur;
            return optTime;
        }

        if (slotsA[idxA][1] > slotsB[idxB][1]) {
            idxB += 1;
        } else {
            idxA += 1;
        }

    }

In your case, you start with 10-50 and 0-15. It did not give enough overlap. Is it worth comparing 0-15 with the other slotA ranges? No, because the others will start at more than 50, and they won't overlap.
Is it worth comparing 10-50 with other slotB ranges? Yes, because there could be a 20-30 range afterwards. Basically, 50 is greater than 15, so thats our hint that we can keep growing that 15 going to next range from slotB and as long as its less than 50, we might keep having an overlap.
If you keep doing this, you will arrive at desired answer.
There might be data structures suited for this, but it is still simple enough to not need one.


Eliminating repetitiveness in code to test hash functions I usually take my school code as a playground to mess around with things. I was given a bunch of premade hash functions and had to test their output and find when they reach a specific lower range. Since the functions all have the same structure, I made a check function that uses a function pointer parameter(I think this is the most efficient way to deal with this problem?). Which I'm iterating through the potential keys to feed to all of the functions it I wanted it to stop at the first one of each, but couldn't think of a way to do so without using 11 individual flags or for loops. I've never messed with bitmasks (I specifically only remember the gist of them because there was a couple commands in quake3 that used them!), but I did that instead and it turned out to not be as pretty/elegant as I thought it would in practice.

in short:

How would you approach the assignment "Using the given 11 hash functions, find each of their first 'low' output?"

void findLowHash();
bool foundLow(unsigned int (*hash)(const std::string &key), std::string key);

void findLowHash() {
  string key;
  key.push_back(0);
  int mask = 2047;
  for (int i = 0; i < 100000; ++i) {
    incrStr(key);
    carryStr(key);
    if (mask == 0)
      break;
    if (mask >> 0 & 1) {
      if (foundLow(RSHash, key)) {
        mask -= 1;
        cout << "RSHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 1 & 1) {
      //cout<<JSHash(key)<<" "<<key<<endl;
      if (foundLow(JSHash, key)) {
        mask -= 2;
        cout << "JSHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 2 & 1) {
      if (foundLow(PJWHash, key)) {
        mask -= 4;
        cout << "PJWHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 3 & 1) {
      if (foundLow(ELFHash, key)) {
        mask -= 8;
        cout << "ELFHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 4 & 1) {
      if (foundLow(BKDRHash, key)) {
        mask -= 16;
        cout << "BKDRHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 5 & 1) {
      if (foundLow(SDBMHash, key)) {
        mask -= 32;
        cout << "SDBMHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 6 & 1) {
      if (foundLow(DJBHash, key)) {
        mask -= 64;
        cout << "DJBHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 7 & 1) {
      if (foundLow(DEKHash, key)) {
        mask -= 128;
        cout << "DEKHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 8 & 1) {
      if (foundLow(FNVHash, key)) {
        mask -= 256;
        cout << "FNVHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 9 & 1) {
      if (foundLow(BPHash, key)) {
        mask -= 512;
        cout << "BPHash"
             << " " << mask << endl;
      }
    }
    if (mask >> 10 & 1) {
      if (foundLow(APHash, key)) {
        mask -= 1024;
        cout << "APHash"
             << " " << mask << endl;
      }
    }
  }
}

bool foundLow(unsigned int (*hash)(const std::string &key), std::string key) {
  if ((*hash)(key) < 10000) {
    std::cout << "key: " << key << " Output:" << ((*hash)(key)) << " ";
    return true;
  }
  return false;
}

Your code does seem open to some improvement. Right now, you're not really getting much good out of using a pointer to a function. You could just about as well invoke each hash function directly in findLowHash as call it via a pointer in foundLow.

In this case, we have a number of functions that all have the same signature, so we can create an array of the pointers to functions. Of course, array should also set off alarm bells--you should almost always prefer std::array or std::vector. In this case, I'll use st::vector, but if you're using a compiler new enough to include it, std::array would be an excellent choice as well.

typedef unsigned int (*hash)(std::string const &key);

std::vector<hash> hashes{
    RSHash, JSHash, PJWHash, ELFHash, BKDRHash,
    SDBMHash, DJBHash, DEKHash, FNVHash, BPHash, APHash
};

Depending on the age of compiler you're using, it might not support the braced init-list like I've used above. At that point, it's open to argument that (under the circumstances) it would be easier to use an actual array instead.

In any case, once you have your pointers to functions in something you can index like array, you can use a loop to walk through the array of functions, and use each function in turn:

for (int h=0; h<hashes.size(); h++)
    for (int i=0; i<100000; i++) {
       incrStr(key);
       carryStr(key);
       if (hashes[h](key) < 10000) {
           std::cout << // ...
           break; // breaks inner loop, so goes to testing next hash function
       }
    }

Again, if you have a new enough compiler (C++11) available, you can make the outer loop a little prettier using a range-based for loop:

for (auto &&hash : hashes)
    // ...
    if (hash(key) < 10000)
        // ...

Once you've done that, there are a few more fairly obvious points, such as replacing the magic numbers like 100000 and 10000 with defined constants with more meaningful names.


Dependency injection with Factory pattern sample code I'm trying to understand and learn about the SOLID principle and especially Dependency Injection (DI) and Factory pattern.

Some background:

I have a production application that uses some "helpers" classes but those are all-in-one classes that I just copy into each new project. One of the (most used) functions is serialization. Our customer has strict demands about history/revision tracing so all important requests are serialized to XML and inserted in Sql server.

With SOLID, DI and Factory in mind I decided to rewrite ye old helpers into a fresh C# project and do things properly.

Below is sample code for Serializers (I have three, XML, JSON and Binary), shown is only the Json class but they are all pretty much the same.

My questions are:

Have I correctly implemented the Factory pattern?
Is the Dependency Injection the way it is supposed to be?

I'm not exactly sure if I even covered both patterns, a mix or perhaps just one?

I hope I made things clear, if not let me know, and thanks for helping me understand DI/Factory better.

Interface

namespace nUtility.Serializers
{
    public interface ISerializer
    {
        string InputString { get; set; }
        object InputObject { get; set; }

        string SerializeToString<T>(); // Need <T> because of the Xml serializer.
        object DeserializeFromString<T>();

    }

}

Factory

namespace nUtility.Serializers
{
    public class SerializersFactory
    {
        readonly ISerializer _iserializer;

        public SerializersFactory(ISerializer iserializer)
        {
            _iserializer = iserializer;
        }

        public string Serialize<T>() => _iserializer.SerializeToString<T>();

        public object Deserialize<T>() => _iserializer.DeserializeFromString<T>();
    }
}

Serializers (Json in this example)

using Newtonsoft.Json;
using System;

namespace nUtility.Serializers
{
    public class nSerializerJson : ISerializer
    {
        public string InputString { get; set; }
        public object InputObject { get; set; }

        public nSerializerJson(string inputString, object inputObject)
        {
            if (inputString == null)
                throw new ArgumentNullException(nameof(inputString));

            if (inputObject == null)
                throw new ArgumentNullException(nameof(inputObject));

            InputObject = inputObject;
            InputString = inputString;

        }

        public string SerializeToString<T>()
        {
            return JsonConvert.SerializeObject(InputObject);
        }

        public object DeserializeFromString<T>()
        {
            return JsonConvert.DeserializeObject<T>(InputString);
        }

    }
}

Program.cs

(SampleValues.InputObjectList is a simple test class which populates a List with 3 items)

var serializersFactory = new SerializersFactory(new nSerializerJson(expectedString, SampleValues.InputObjectList));
expectedString = serializersFactory.Serialize<List<SerializeTestObject>>();

Two more things:

Why not static class for serializers - I kinda agree with this answer on SO, however on a second thought changing my class (the parameter creep/constructor point in SO answer) would violate SOLID (the SRP) so I'm not sure if this is correct decision?

I know I'm breaking the Type and Namespace naming convetion by putting "nSerializers" etc. but this "n" prefix has been in the company for 9 years now and we're not prepared to let go yet :).

public class SerializersFactory
{
    readonly ISerializer _iserializer;

    public SerializersFactory(ISerializer iserializer)
    {
        _iserializer = iserializer;
    }

    public string Serialize<T>() => _iserializer.SerializeToString<T>();

    public object Deserialize<T>() => _iserializer.DeserializeFromString<T>();
}

Sorry to burst your bubble, but... this isn't a factory. In fact, it's rather confusing what purpose it serves, given how that serializer is implemented:

    public nSerializerJson(string inputString, object inputObject)
    {
        if (inputString == null)
            throw new ArgumentNullException(nameof(inputString));

        if (inputObject == null)
            throw new ArgumentNullException(nameof(inputObject));

        InputObject = inputObject;
        InputString = inputString;
    }

It's very confusing why you need to specify both the serialized JSON string and the deserialized object for the constructor to not throw an ArgumentNullException. Actually, it's very hard to understand why one would even need this constructor.

It's the methods that need the parameters; having them at instance-level seems to make the whole thing more or less useless - and confusing.

What's the role of the <T> type parameter here?

public string SerializeToString<T>()
{
    return JsonConvert.SerializeObject(InputObject);
}

Oh, I see:

string SerializeToString<T>(); // Need <T> because of the Xml serializer.

Since when do implementations dictate what abstractions look like? That T is clearly begging for a method parameter there, of type T.

Seems you need to stop and ask yourself what you're trying to accomplish, and why. Your system has a dependency on Newtonsoft.Json; your serializer is essentially wrapping the Newtonsoft API... in some awkward way.

I would have used something like this:

public interface ISerializer
{
    string Serialize<T>(T deserialized);
    T Deserialize<T>(string serialized); // <out T>
}

And the implementation would be something like this:

public class JsonSerializer : ISerializer
{
    public string Serialize<T>(T deserialized)
    {
        return JsonConvert.SerializeObject(deserialized);
    }

    public T Deserialize<T>(string serialized)
    {
        return JsonConvert.DeserializeObject<T>(serialized);
    }
}

What else would you need? I think everything else you got here, is over-engineered fluff that can be removed. There's nothing to DI here, ISerializer is the dependency.

As for the factory, there's no need for one, since any type that needs an ISerializer can just tell the IoC container it needs one, by specifying an ISerializer parameter in its constructor.


Handle API timeouts in Node Sails.js I am working on a node.js sails.js app that handles a number of API calls and uses Bluebird for promises. The API calls have been reliable but I would like to build in handling for if they do not respond. I'm wondering if there is a good standard practice for handling timeouts (or a different approach) if the API services are down or do not respond.

Here is the code I have found/modified to work with, 15000 is a 15 second timeout on the API. Is this approach good, is there a better way?

var Promise = require('bluebird');

return new Promise(function (resolve, reject) {
  //resolve my API call here.
})
.timeout(15000)
.cancellable()
.catch(Promise.CancellationError, function(error) {
  // ... must neatly abort the task ...
  console.log('Task cancelled', error);
})
.catch(Promise.TimeoutError, function(error) {
  // ... must neatly abort the task ...
  console.log('Task timed out', error);
});

The main issue I have with this code is that you're calling .timeout() on a Promise instance; by default, Promise doesn't support this - this is a Bluebird only operation.

This means that your returned Promise must evaluate true for Promise <: BluebirdPromise, which is not something that can be guaranteed in all environments (especially if you do what you've done here and omit the declaration for require('bluebird').

I'd prefer to see something like this:

const BluebirdPromise = require('bluebird');

return new BluebirdPromise(function (resolve, reject) {
  //resolve my API call here.
})
.timeout(15000)
.cancellable()
.catch(BluebirdPromise.CancellationError, function(error) {
  // ... must neatly abort the task ...
  console.log('Task cancelled', error);
})
.catch(BluebirdPromise.TimeoutError, function(error) {
  // ... must neatly abort the task ...
  console.log('Task timed out', error);
});

At the very least, this makes it clear that you aren't using the standard ES6 Promise, but that you're using a specific version from Bluebird.

I've not used the timeout and cancellable methods, so I can't speak for how you could code that better specifically, so there's probably more to say here.


iterating over the values of a list of ordered dictionaries I am facing the following problem: I have a special data structure that is a dictionary whose keys are integers (dimensions). The values are also dictionaries whose keys are strings (geometric types) and whose values are numpy arrays (connectivities). Something like this:

custom = {1: {'a': np.zeros(10), 'b': np.zeros(100)}, 2:{'c': np.zeros(20), 'd': np.zeros(200)}}

I use this data structure quite a lot in the code, and every time I need to iterate over all rows in the numpy arrays of this data structure (all values of the dictionaries), I have to type:

for d, delem in custom.items():
    for k, v in delem.items():
        for row in v:
            print(row)

And this is way too verbose. So to make things easier to other developers, I am trying to encapsulate this behavior in a custom class so that I can type:

for row in custom:
    print(row)

Also, an important requirement is that all rows have to be traversed "in order".
And so I came up with a Test class that derives from list. The class is initialized by passing a dictionary (maybe I can improve this later by just adding the elements directly to it). I have overridden the __iter__ and __next__ methods to provide the iteration type I want. By default, iteration is done through the highest dimension present. But the user can do iteration over lower-dimensional dictionaries and for that I overrode the __call__ method.
The technique works, I an iterate as many times as I want over this data structure, and I accomplished the simplicity I wanted. But I'm a bit concerned about performance, as I don't think that calling self[self.d][self.etype][self.idx-1] is very efficient. Maybe there are other improvements to be made as well that I don't see.

import sys
import numpy as np
from collections import OrderedDict

custom = {0: {'n1': np.array([[3]])}, 1: {'l2': np.array([[ 0,  4],
       [ 4,  5],
       [40, 41],
       [41, 42],       [57,  3],
       [57,  3]])}, 2: {'t3x': np.array([[188, 401, 400],
       [188, 187, 401],
       [187, 205, 401],
       [324, 306, 417],
       [306, 305, 417],
       [305, 416, 417]]), 'q3': np.array([[188, 401, 400, 0],
       [188, 187, 401, 0],
       [187, 205, 401, 0],
       [323, 324, 417, 0],
       [324, 306, 417, 0],
       [306, 305, 417, 0],
       [305, 416, 417, 0]])}}

class Test(list):

    def __init__(self, *args, **kwargs):

        super(Test, self).__init__(*args, **kwargs)

        for k,v in args[0].items():
            self[k] = OrderedDict(v)

        self.d = -1
        self.setup()

    def setup(self):
        self.iterator = iter(self[self.d].keys())
        self.etype = next(self.iterator)
        self.idx = 0

    def __iter__(self):
        self.setup()
        return self

    def __next__(self):

        try:
            self.idx += 1
            return self[self.d][self.etype][self.idx-1]

        except IndexError:

            self.etype = next(self.iterator)
            self.idx = 0
            return self[self.d][self.etype][self.idx-1]

    def __call__(self, d):
        self.d = d-1
        return self

def main(argv=()):

    tst = Test(custom)
    print(tst)

    # iterate over the container
    for el in tst:
        print(el)

    # iterate again over the container
    for el in tst:
        print(el)

    # iterate over lower dimension
    for el in tst(-1):
        print(el)

    return 0

if __name__ == "__main__":
    sys.exit(main())

yield!

You're way overthinking the problem. Python has yield. This is one of the coolest things in Python (IMHO). When you want to iterate over a container like this:

for d, delem in custom.items():
    for k, v in delem.items():
        for row in v:
            print(row)

You just write a function that looks pretty much exactly like that:

def iter_over_custom(custom):     ## or a better name
    for _, delem in custom.items():
       for _, v in delem.items():
           for row in v:
               yield row

We can even drop the last loop if you have a sufficiently recent version of Python that supports yield from:

def iter_over_custom(custom):     ## or a better name
    for _, delem in custom.items():
       for _, v in delem.items():
           yield from v

Or you don't actually need .items() since you just need the values:

def iter_over_custom(custom):
    for delem in custom.values():
        for v in delem.values():
            yield from v

That's it. No need for a custom class to effectively reimplement the same. The initial loop could them be written as:

for row in iter_over_custom(custom):
    print(row)

Note that if you had your own class, you could write the above in __iter__ too:

class Custom(object):
    ...
    def __iter__(self):
        for delem in self.whatever.values():
            for v in delem.values():
                yield from v
    ...


Two-layer RAII to support a critical section For the code, if one considers using RAII to encapsulate CriticalSection (global scope), two RAII layers are needed: InitializeCriticalSection layer (main function scope), enter EnterCriticalSection layer (multi-thread based Proc scope).

// Global variable
CRITICAL_SECTION CriticalSection;

int main( void )
{
    // Initialize the critical section one time only.
    if (!InitializeCriticalSection(&CriticalSection) )
        return;
    ...

    // Release resources used by the critical section object.
    DeleteCriticalSection(&CriticalSection);
}

DWORD WINAPI ThreadProc( LPVOID lpParameter )
{
    // Request ownership of the critical section.
    EnterCriticalSection(&CriticalSection);

    // Access the shared resource.

    // Release ownership of the critical section.
    LeaveCriticalSection(&CriticalSection);

    return 1;
}

As CriticalSection and the two layers all have different scopes, and also enter InitializeCriticalSection layer must be on top of EnterCriticalSection layer.

class CriticalSectionGuard {
public:
    CriticalSectionGuard(CRITICAL_SECTION * cs) : m_CS(cs) {
        InitializeCriticalSection(m_CS);
    }
    ~CriticalSectionGuard() {
        DeleteCriticalSection(m_CS);
    }
    void lock() {
        EnterCriticalSection(m_CS);
    }
    void unlock{
        LeaveCriticalSection(m_CS);
    }
private:
    CRITICAL_SECTION * m_CS;
}

class GetLock {
public:
    GetLock(CriticalSectionGuard* plock) : m_csg(plock) {
        m_csg->lock();
    }
    ~GetLock(void) {
        m_csg->unlock();
    }
private:
    CriticalSectionGuard * m_csg;
};

// Global variable
CRITICAL_SECTION CriticalSection;

int main(void)
{
    // Initialize the critical section one time only.
    CriticalSectionGuard CSG(&CriticalSection);
    ...
}
DWORD WINAPI ThreadProc(LPVOID lpParameter)
{
    // we need pass along the CSG Pointer, which is messy.
    GetLock(&CSG);
    ...

        return 1;
}

It looks quite bad, as the CriticalSectionGuard object is not in the global scope, it need pass along to threads.

Please share any suggestions or criticisms to improve the code.

We can simplify that a lot.

You are not really using RAII on the CriticalSection as you declare it in one place then initialize it in another. But CRITICAL_SECTION is a type as you pass a pointer around.

Why not simplify a bit:

class GetLock;
class CriticalSection {
public:
    CriticalSection() {
        InitializeCriticalSection(&m_CS); // get a pointer to the member
    }
    ~CriticalSection() {
        DeleteCriticalSection(&m_CS);
    }
private:
    friend class GetLock;   // Only GetLock should be locking/unlocking this object.
    void lock() {
        EnterCriticalSection(&m_CS);
    }
    void unlock{
        LeaveCriticalSection(&m_CS);
    }
private:
    CRITICAL_SECTION    m_CS;   // Make this a member
                                // Now make the constructor initialize it.
}

Now its use case becomes.

CriticalSection  criticalSection;

int main() {
    // Critical section variable already up and running.
}

If we look at the GetLock.
I would not passes pointers as parameters. This means people can accidentally pass a nullptr and thus cause your code to break. Always pass by reference if you have to have parameter there.

class GetLock {
public:
    // Pass a reference to the CriticalSection
    GetLock(CriticalSection& plock)
        : m_csg(plock)
    {
        m_csg.lock();
    }
    ~GetLock(void) {
        m_csg.unlock();
    }
private:
    CriticalSectionGuard& m_csg;   // Keep a reference to the critical section.
};

Now this can be used like:

DWORD WINAPI ThreadProc(LPVOID lpParameter)
{
    GetLock lock(criticalSection);

    return 1;
}

Note the difference above to your version:

    GetLock(&CSG);   // This creates a temporary object.
                     // This object is destroyed at the end of the
                     // statement (which is the ;).
                     // This mean you call lock() and unlock() before
                     // the end of the line.

I should also note that using global variables is considered bad practice. You should try and design your program so that you pass the things being manipulated around as parameters (or part of an object). I can't really suggest an improvement with knowing more about the code.


Policemen catch thieves Given an array of size n that has the following specifications:

  Each element in the array contains either a policeman or a thief.

  Each policeman can catch only one thief.

  A policeman cannot catch a thief who is more than K units away from the policeman.

  We need to find the maximum number of thieves that can be caught.

Here's code for policemen catch thieves written in Python3, I would like feedback/suggestions.

import random

def tp_build(n):
    thief_police = []
    for i in range(n):
        thief_police.append(random.choice(['T', 'P']))
    return thief_police

def catch(k=2, n=10):
    tp_list = tp_build(n)
    print('List: %s' % tp_list)
    if 'P' not in tp_list:
        return 'No Police!'
    if 'T' not in tp_list:
        return 'No Thieves!'
    p_indexes = []
    t_indexes = []
    for i in range(n):
        if tp_list[i] == 'P':
            p_indexes.append(i)
        elif tp_list[i] == 'T':
            t_indexes.append(i)
    combinations = []
    for limit in range(1, k + 1):
        for police in p_indexes:
            if police + limit in t_indexes:
                combinations.append((police, police + limit))
        for police in p_indexes:
            if police - limit in t_indexes:
                combinations.append((police, police - limit))
    p_list = []
    t_list = []
    for i, j in combinations:
        p_list.append(i)
        t_list.append(j)
    new_p = []
    new_t = []
    for i in p_list:
        if i not in new_p:
            new_p.append(i)
    for j in t_list:
        if j not in new_t:
            new_t.append(j)
    final_combinations = list(zip(new_p, new_t))
    print('Number of thieves caught: %s'%(len(final_combinations)))
    return len(final_combinations)

if __name__ == '__main__':
    for i in range(100):
        catch()

Bugs

You return 'No Police!' instead of returning 0, but you don't check the return value. You might consider printing the string and returning 0 instead. Likewise for the no-thieves case.

Style

You are nominally compliant with pep-8, but your style is very much a beginner style. Why do you have variables named t_list and p_list? Do you really think that the _list part of the name is the most important, and likely to be overlooked by readers?

When choosing a name, you should almost always avoid using the type of the data in the name (1). After all, the type might change! (For example, you might switch from a list to a dict for storage.)

(1): Except when the type is the important part, like if you're writing a type conversion function (str2int or wav2mp3 or something).

Iteration

You are missing some tricks of Python iteration. First, let's look at your tp_build function:

def tp_build(n):
    thief_police = []
    for i in range(n):
        thief_police.append(random.choice(['T', 'P']))
    return thief_police

This code is a one-off "helper" that does a job that is described in the problem statement: generate a random array of police & thieves. Because it's simple and the requirements are clearly explained by the problem statement, you don't need to worry about documenting this, or leaving the code spelled out for comprehensibility. So just use a list comprehension and shorten that code a lot (also, strings are iterable):

def tp_build(n):
    return [random.choice("TP") for _ in range(n)]

In your ``catch` function, you have a lot of iteration. Again, you're writing too-simple loops when you could get clarity and performance from Python features.

p_indexes = []
t_indexes = []
for i in range(n):
    if tp_list[i] == 'P':
        p_indexes.append(i)
    elif tp_list[i] == 'T':
        t_indexes.append(i)

In this paragraph, you construct lists of the indexes of the different characters. If you're going to look up the value of tp_list[i], you should just use enumerate:

p_indexes = []
t_indexes = []
for i, ch in enumerate(tp_list):
    if ch == 'P':
        p_indexes.append(i)
    else:
        t_indexes.append(i)

Of course, this is another case where you could use comprehensions. In theory, this is twice as slow. In practice, the comprehension might be faster because Python knows how to speed it up. You'll have to check it:

p_indexes = [i for i in range(n) if tp_list[i] == 'P']
t_indexes = [i for i in range(n) if tp_list[i] == 'T']

Clarity

You don't provide a function level comment explaining your algorithm, and frankly it's not obvious to me. Obviously you are pairing some police with some thieves. But it would be helpful if you explained what/how your pairing mechanic was, and why you felt it produced a best-possible result. In cases where there are potential conflicts between pairings, how do you decide which to choose, and why?

Also, note that at the end you are doing a lot of unnecessary work.

new_p = []
new_t = []
for i in p_list:
    if i not in new_p:
        new_p.append(i)
for j in t_list:
    if j not in new_t:
        new_t.append(j)
final_combinations = list(zip(new_p, new_t))
print('Number of thieves caught: %s'%(len(final_combinations)))
return len(final_combinations)

First, you make new_p and new_t. (Which aren't new: bad names.) You make them contain unique members of a previous list. Then you zip them together. The effect of zip is to stop when the shortest source iterable is exhausted. Then you compute the len of the result.

Effectively, you're asking for the length of the shortest unique group of indexes. Since order really doesn't matter here (you're using the len of the result) you can use a set. And since you just care about the length, you don't need to zip them, just compare their lengths:

catches = min(len(set(p_list)), len(set(t_list)))
print('Number of thieves caught: %d' % catches)
return catches


Pagination filtering I want to check if there is page variable (in URL) and if it is, if it's correct - it cannot be number 1 because this is default page and it should be valid int from 2 to max number of pages (not octal and not hexadecimal).

<?php

$_GET['page'] = '01'; // this line is only for testing

$selPage = 1;
$totalPages = 10;

if (isset($_GET['page'])) {
    $selPage = filter_var($_GET['page'], FILTER_VALIDATE_INT, ['options' => ['min_range' => 2, 'max_range' => $totalPages]]);
    if ($selPage === false) {
        exit('redirection'); // this line for test only - in fact make 301 redirection here to correct url
    }
}
echo $selPage;

Could it be improved anyway, or is it the best and shortest it can be?

Important:
Asking for the "best and shortest" code is a contradiction. If you write something as short as possible, chances are you're taking short-cuts in terms of error handling, maintainability, and readability. Take, for example these two snippets of code. Both do exactly the same thing, but one is so horribly obfuscated that nobody in their right mind would consider it to be good code:

$a = range('a', 'z');
for ($a=range('a','z'),$i=0,$j=count($a),$s='',$k=mt_rand()%j;$i<$j;++$i, unset($a[$k]))
{
    while (!isset($a[$k])) $k = mt_rand()%j;
    $s .= $a[mt_rand()%$j];
}
//compared to
$chars = range('a', 'z');//array of all chars to shuffle
$len = count($chars);//get the length, avoid calling count too many times
$string = '';//the string containing the chars in random order
for ($i=0;$i<$len;++$i)
{
    do
    {//keep generating a random key, until we find one that exists
        $k = mt_rand()%$len;
    } while(!isset($chars[$k]));
    $string .= $chars[$k];//add char to string
    unset($chars[$k]);//unset the char
}

Now I realize that this code is really rather pointless, but it's simply to prove a point. Which of these two snippets of code would you rather be confronted with, when trying to debug something? Or which snippet of code would be easier to explain? I don't know you, but assuming you're not a masochist, I'm going to assume you prefer the second version...
It contains comments, explaining what bit of code does what, it uses (more or less) meaningful variable names, and most of all, there isn't a single line of code that does more than it needs to. Cramming everything in one statement is the shortest route to introducing odd bugs, and because you've crammed everything into a one-liner, those bugs can prove tricky to fix.

Anyway, let's look at your code:

You have a fundamental flaw in your code: even if the page parameter is set, and is valid, the first statement of your code is overwriting it. Suppose the page parameter is 4, the first thing you do is:

$_GET['page'] = '01';//<-- reassignment

There's no way you'll ever get the actual requested page. I'd suggest you write a function to get the correct page number:

/**
 * @param array $params
 * @param string $key
 * @param array $filter
 * @param mixed $default = null
 * @return mixed
 * @throws \InvalidArgumentException
 */
function getValue(array $params, $key, array $filter, $default = null)
{
    if (!$params || !isset($params[$key])
        return $default;//no GET params, or key is not set, return default
    $value = $params[$key];//use the raw value
    //validate
    $validated = filter_var($value, $filter['type'], $filter['options']);
    if ($validated === false)
    {//validation failed
        throw new \InvalidArgumentException(
            sprintf(
                '%s is an invalid value',
                $value
            )
        );
    }
    return $validated;
}
//usage:
$page = getValue(
    $_GET,
    'page',
    array(
    'type' => FILTER_VALIDATE_INT,
    'options' => array(
         'min_range' => 1,//1 is valid, if user was on page 2, and requests page 1 again
         'max_range' => 10
    ),
    1//default value
);

Wrap this call in a try-catch block, and redirect the user if the exception is thrown. The benefit of this function is that you can easily re-use this function to validate other types of data:

$email = getValue(
    $_POST,
    'email',
    array(
    'type' => FILTER_VALIDATE_EMAIL,
    'options' => null
);
if ($email === null)
    //handle $_POST['email'] not set error

At first glance, this code seems slightly more verbose, but not much, and it's a "price" worth paying considering the benefits:

Reusable function
one call to validate $_GET, $_POST, $_COOKIE and $_SESSION keys
Can be used on any array (DB results, API responses)
Easy error handling (catch the exception, handle default returns if required)
If argument not set, get a usable default value back (like passing 1 in the page example as default value).
Maintainable code
This function might well be the first step towards building your own Request class.

But all in all, I'd suggest you look into existing paginator tools. There are plenty of them, some of them are really quite good, like the KnpLabs paginatorBundle.


Parsing JSON string from HTTP request HTTP request is made, and a JSON string is returned, which needs to be parsed.
Example response:

{"urlkey": "com,practicingruby)/", "timestamp": "20150420004437", "status": "200", "url": "https://practicingruby.com/", "filename": "common-crawl/crawl-data/CC-MAIN-2015-18/segments/1429246644200.21/warc/CC-MAIN-20150417045724-00242-ip-10-235-10-82.ec2.internal.warc.gz", "length": "9219", "mime": "text/html", "offset": "986953615", "digest": "DOGJXRGCHRUNDTKKJMLYW2UY2BSWCSHX"}
{"urlkey": "com,practicingruby)/", "timestamp": "20150425001851", "status": "200", "url": "https://practicingruby.com/", "filename": "common-crawl/crawl-data/CC-MAIN-2015-18/segments/1429246645538.5/warc/CC-MAIN-20150417045725-00242-ip-10-235-10-82.ec2.internal.warc.gz", "length": "9218", "mime": "text/html", "offset": "935932558", "digest": "LJKP47MYZ2KEEAYWZ4HICSVIHDG7CARQ"}
{"urlkey": "com,practicingruby)/articles/ant-colony-simulation?u=5c7a967f21", "timestamp": "20150421081357", "status": "200", "url": "https://practicingruby.com/articles/ant-colony-simulation?u=5c7a967f21", "filename": "common-crawl/crawl-data/CC-MAIN-2015-18/segments/1429246641054.14/warc/CC-MAIN-20150417045721-00029-ip-10-235-10-82.ec2.internal.warc.gz", "length": "10013", "mime": "text/html", "offset": "966385301", "digest": "AWIR7EJQJCGJYUBWCQBC5UFHCJ2ZNWPQ"}

My code:

result = Net::HTTP.get(URI("http://index.commoncrawl.org/CC-MAIN-2015-18-index?url=#{url}&output=json")).split("}")

result.each do |res|
    break if res == "\n"
    #need to add back braces because we used it to split the various json hashes from the http request
    res << "}"
    to_crawl = JSON.parse(res)
    puts to_crawl
end

It works, but I'm sure there is a much better way to do it, or at least a better way to write the code.

This body.split('{'}) is doing you a disservice, as it destroys the structure of the response. Split it by lines instead:

body = Net::HTTP.get(...)
data = body.lines.map { |line| JSON.parse(line) }


Finding numbers with given gcd and lcm I am solving a question in which we are given the LCM and HCF and two numbers 'a' and 'b', our task is to find the pairs of numbers between a and b [inclusive] whose HCF is the given HCF and LCM is the given LCM

I have written a code with time complexity O(m) where m is the number of integers lying between the range a and b, But on submission i am getting time limit exceeded error

IMO this is the best we can do , can we do better?
Here is the code

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

  //function for calculatin gcd
  int gcd( int a,int b){
  int h;
   for(h = a<b?a:b; h >= 1; h--){
    if((a%h == 0) && (b%h == 0))
     break;
   }

   return h;
   }

  int main(){
  int n1,n2,lcm,hcf,cnt = 0;
  cin>>n1>>n2>>hcf>>lcm;//The numbers a,b and hcf and lcm
   for(int i = n1; i <= n2;i++){
   //checking if the number divides the lcm completely
   if(lcm%i == 0){
    int a = lcm/i;// its other multiplier
    int b = gcd(a,i);//finding their gcd
        //checking if their gcd is equal to given hcf
        if(b == hcf){
         int c = (i*a)/b;//finding the lcm by a*b = lcm*hcf formula and
                         //checking
          if(c == lcm){
           cnt++;
          }
         }
        }
       }

   cout<<cnt<<endl;
   return 0;
   }

Headers

Use the standard headers, not #include<bits/stdc++.h>. This is intended solely for gcc's internal use, and gratuitously renders your code completely non-portable.

Oh No! Not using namespace std;!

Please don't use using namespace std; It can, will, and does declare an unknown and almost unbounded set of names, many of which 1) you may not know about (it grows constantly), and 2) many of which are really common names that can collide with all sorts of reasonable code, leading to no end of potential problems.

Use the Library

If you're using a current compiler, you probably have std::gcd defined in <numeric>, so you probably don't need to define your own.

Improved Algorithm

To fit the criteria, each number you try must be a multiple of the given gcd (otherwise that wouldn't be the gcd of the number in question). As such, you can start from the smallest number that's a multiple of the gcd, but greater than your lower bound.

For example, if you're given a gcd of 21 and a lower bound of 30, you can quickly find that your starting point is 42, because that's the next number larger than 30 that's also a multiple of 21.

From there, you have to multiply each candidate by numbers that are coprime with each other--if the multipliers weren't coprime, then the common factors would become part of the gcd, and the gcd of the two would no longer be what you were given. So, for example, given a gcd of 21, you could try 21*2 = 42 with 21*3 = 63. Since 2 and 3 are coprime, that would gcd(42,63) would still be 21 (as required).


Solving 15Puzzle with Julia Asking here instead of SO as suggested.
I'm trying to use Julia to solve the common tile game 15 Puzzle using Julia using A* algorithm. I am quite new to the language and my style may seem very C like. When I try the following code, I run out of memory. I'm not sure if its related to the use of a pointer style in my structs or just bad design.
struct Node
    parent
    f::Int64
    board::Array{Int64,1}
end

function findblank(A::Array{Int64,1})
    x = size(A,1)
    for i = 1:x
        if A[i] == x
            return i
        end
    end
    return -1
end

function up(A::Array{Int64,1})
    N = size(A,1)
    Nsq = isqrt(N)
    blank = findblank(A)
    B = copy(A)
    if blank / Nsq <= 1
        return nothing
    end
    B[blank-Nsq],B[blank] = B[blank],B[blank-Nsq]
    return B
end

function down(A::Array{Int64,1})
    N = size(A,1)
    Nsq = isqrt(N)
    blank = findblank(A)
    B = copy(A)
    if (blank / Nsq) > (Nsq -1)
        return nothing
    end
    B[blank+Nsq],B[blank] = B[blank],B[blank+Nsq]
    return B
end

function left(A::Array{Int64,1})
    N = size(A,1)
    Nsq = isqrt(N)
    blank = findblank(A)
    B = copy(A)
    if (blank % Nsq) == 1
        return nothing
    end
    B[blank-1],B[blank] = B[blank],B[blank-1]
    return B
end

function right(A::Array{Int64,1})
    N = size(A,1)
    Nsq = isqrt(N)
    blank = findblank(A)
    B = copy(A)
    if (blank % Nsq) == 0
        return nothing
    end
    B[blank+1],B[blank] = B[blank],B[blank+1]
    return B
end

function manhattan(A::Array{Int64,1})
    N = size(A,1)
    Nsq = isqrt(N)
    r = 0
    for i in 1:N
        if (A[i]==i || A[i]==N)
            continue
        end
        row1 = floor((A[i]-1) / Nsq)
        col1 = (A[i]-1) % Nsq
        row2 = floor((i-1) / Nsq)
        col2 = (i-1) % Nsq
        r+= abs(row1 - row2) + abs(col1 - col2)
    end
    return r
end

# start = [1,2,3,4,5,6,7,9,8]
# start = [6,5,4,1,7,3,9,8,2] #26 moves
start = [7,8,4,11,12,14,10,15,16,5,3,13,2,1,9,6] # 50 moves
goal = [x for x in 1:length(start)]
# println("The manhattan distance of $start is  $(manhattan(start))")
g = 0
f = g + manhattan(start)
pq = PriorityQueue()
actions = [up,down,left,right]
dd = Dict{Array{Int64,1},Int64}()
snode = Node(C_NULL,f,start)
enqueue!(pq,snode,f)
pos_seen = 0
moves = 0
while (!isempty(pq))
    current = dequeue!(pq)
    if haskey(dd,current.board)
        continue
    else
        push!(dd, current.board =>current.f)
    end
    if (current.board == goal)
        while(current.board != start)
            println(current.board)
            global moves +=1
            current = current.parent[]
        end
        println(start)
        println("$start solved in $moves moves after looking at $pos_seen positions")
        break
    end
    global pos_seen+=1
    global g+=1
    for i in 1:4
        nextmove = actions[i](current.board)
        if (nextmove === nothing || nextmove == current.board || haskey(dd,nextmove))
            continue
        else
            global f = g+manhattan(nextmove)
            n = Node(Ref(current),f,nextmove)
            enqueue!(pq,n,f)
        end
    end
end
println("END")

It looks like you store the board after each movement for each possibility, that's a lot of arrays in memory, no wonder it fills your memory
for your second example, your code looks for 157523 positions, which is half of the total possibilities.
the number of permutations for 1:16 is enormous, the a-star algorithm is probably not sufficient
even if you look at only 1% of the total possibilities, you would need hundreds of gigabytes if not terabytes to store them
[6, 5, 4, 1, 7, 3, 9, 8, 2] solved in 26 moves after looking at 157523 positions

julia> using Combinatorics

julia> length(permutations(1:9))
362880

julia> length(permutations(1:16))
20922789888000


Merge n sorted iterators Over the weekend I was curious about efficiently merging multiple sorted iterators together, and for them to be in sorted order.
This is quite like a challenge on HackerRank:

  You’re given the pointer to the head nodes of two sorted linked lists. The data in both lists will be sorted in ascending order. Change the next pointers to obtain a single, merged linked list which also has data in ascending order. Either head pointer given may be null meaning that the corresponding list is empty.

I may have cheated a bit by printing, rather than returning the items. However I don't mind that much about the HackerRank challenge.

I managed to do this in about \$O(3l)\$ space and \$O(l(2+n))\$ time. So \$O(l)\$ and \$O(ln)\$. Where \$l\$ is the amount of lists, and \$n\$ is the amount of data.

import operator
import functools

def from_iterable(fn):
    @functools.wraps(fn)
    def inner(*args):
        return fn(args)
    inner.from_iterable = fn
    return inner

@from_iterable
def merge(sources):
    sources = {
        id_: iter(source)
        for id_, source in enumerate(sources)
    }
    values = {}
    for id_, source in list(sources.items()):
        try:
            values[id_] = next(source)
        except StopIteration:
            del sources[id_]

    by_value = operator.itemgetter(1)
    while sources:
        id_, minimum = min(values.items(), key=by_value)
        try:
            values[id_] = next(sources[id_])
        except StopIteration:
            del values[id_], sources[id_]
        yield minimum

def iter_nodes(node):
    while node is not None:
        yield node.data
        node = node.next

def MergeLists(headA, headB):
    vals = merge.from_iterable(iter_nodes(l) for l in (headA, headB))
    print(' '.join(str(i) for i in vals), end='')

A bit overkill for the HackerRank challenge. But that wasn't the main reason for doing it.
Any and all reviews are welcome.

There are no docstrings. What do these functions do? What arguments do they take? What do they return?
Decorators are normally named after the effect they have on the decorated function. For example @functools.lru_cache adds an LRU cache to a function; @unittest.skip causes a test case to be skipped, and so on.

The effect of @from_iterable is that it converts the arguments into a tuple and passes that tuple as a single argument to the original function. I would not have guessed this functionality from the name; in fact I would have guessed the reverse (that it would convert a single iterable argument to individual arguments).
It was established in comments that the intention of this decorator is to transform a function into a pair of functions like chain and chain.from_iterable (where the original function becomes available under the latter name, and the new function under the former name).

The trouble with this is that chain was a mistake. If we only had chain.from_iterable, then we would not need chain, because we could just make a tuple: we'd write chain.from_iterable((arg1, arg2, ...)) instead of chain(arg1, arg2, ...). This is a trivial change to calling convention (just two parentheses) that does not need another function to support it. (For example, it would be ridiculous to have sorted and sorted.from_iterable.)

The Python developers were constrained by backwards compatibility to leave chain alone, and so the only thing they could do was add another function. I don't think they intended it to be a pattern to be followed (it's certainly not used anywhere else in the standard library). To this day, Python programmers looking for a "flatten" function are surprised to find that it already exists in the standard library under an obscure name.

Note that the code in the post does not actually use merge, it only uses merge.from_iterable. So the obvious thing to do would be to write merge so that it takes an iterable, and avoid the need for the @from_iterable decorator and its associated confusion.
Since headA and headB are not used independently of each other, you could change the calling convention for MergeLists so that it took an arbitrary number of linked lists, like this:

def merge_linked_lists(*lists):
    "Merge multiple sorted linked lists."
    return merge(map(iter_nodes, lists))

The expression:

str(i) for i in vals

can be written:

map(str, vals)

Or if you prefer the comprehension, then consider improving the name of the loop variable. The name i is conventionally used for an index, but here we are not iterating over indexes, but over values, so v or value would be clearer.
The code in merge does not use the built-in function id, so there is no need to respell the variable as id_ in order to avoid shadowing the built-in.
The function merge maintains two data structures. The dictionary source maps a source id to an iterator over the corresponding source, and the dictionary values maps a source id to the value at the front of the corresponding source. The code would be simplified if the two data structures were combined into one, like this:

def merge(iterables):
    "Merge sorted iterables."
    entries = {} # Map from id to [front value, id, iterator].
    for id, it in enumerate(map(iter, iterables)):
        try:
            entries[id] = [next(it), id, it]
        except StopIteration:
            pass
    by_value = operator.itemgetter(1)
    while entries:
        id, entry = min(entries.items(), key=by_value)
        value, _, it = entry
        yield value
        try:
            entry[0] = next(it)
        except StopIteration:
            del entries[id]

The runtime bottleneck is the call to min to find the entry with the smallest value. This takes time proportional to the number of remaining iterables, that is, \$O(l)\$.

We can improve this to \$O(\log l)\$ if we keep the entries in a heap, using the heapq module:

from heapq import heapify, heappop, heapreplace

def merge(iterables):
    "Merge sorted iterables."
    entries = []          # Heap of [front value, id, iterator].
    for id, it in enumerate(map(iter, iterables)):
        try:
            entries.append([next(it), id, it])
        except StopIteration:
            pass
    heapify(entries)
    while entries:
        value, _, it = entry = entries[0]
        yield value
        try:
            entry[0] = next(it)
            heapreplace(entries, entry)
        except StopIteration:
            heappop(entries)

This improves the overall runtime to \$O((n + l)\log l)\$.
This is already built into Python as heapq.merge. It's worth taking a look at the source code — the code in §8 above is a simplified version of this.