Aligning out-of-sync subtitles This is my 2nd program I wrote on my own, a pretty simple program consisting of two functions (those functions are not combined in any way yet though). The first function moves the display times of subtitles saved in .srt files by a specified amount of time.

The second function "warps" the display times by a specified multiplier to fix subtitles written on the base of sped up/slowed down versions of the video(which results in the subtitles syncing out after some time).

Everything works fine and as intended, but I'm pretty certain there are things I could have done more optimally.

For starters, the argument input could be improved, so that you can just input "+20" instead of "+20:00,000" to shift the subtitles by 20 seconds.

That's pretty much the only problem I can come up that I don't know how to solve right now. Of course there are probably more problems with the code I can't spot yet, so any input is appreciated.

import re; import datetime; import time

def timeChangeSRT(n, s):
    t = time.time()
    f, new = open(n, 'r'), open('new' + n, 'w')
    offset = re.search('([\+\-])?((\d{1,2})\:)?(\d+)(\,(\d{1,3}))?', s)
    offset = datetime.timedelta(minutes=int(s[1:3]), seconds=int(s[4:6]), milliseconds=int(s[7:10]))

    for line in f:
        parsed = re.search('(\d{2}:\d{2}:\d{2}.\d{3}) \-\-> (\d{2}:\d{2}:\d{2}.\d{3})', line)
        if parsed:
            start = datetime.timedelta(minutes=int(parsed.group(1)[3:5]), \
                                       seconds=int(parsed.group(1)[6:8]), \
                                       milliseconds=int(parsed.group(1)[9:12]))
            end = datetime.timedelta(minutes=int(parsed.group(2)[3:5]), \
                                       seconds=int(parsed.group(2)[6:8]), \
                                       milliseconds=int(parsed.group(2)[9:12]))
            if '-' in s:
                start -= offset
                end -= offset
                newline = '0' + str(start)[:11] + " --> " + '0' + str(end)[:11] + '\n'
                new.write(newline)
            else:
                start += offset
                end += offset
                newline = '0' + str(start)[:11] + " --> " + '0' + str(end)[:11] + '\n'
                new.write(newline)

        else:
            new.write(line)
    stop = time.time() - t
    print('The %s file has been updated. New .srt file named %s has been created. The process took %s seconds.' % (n, 'new' + n, stop))

def warpSRT(n, w):
    t = time.time()
    f, new = open(n, 'r'), open('new' + n, 'w')
    offset = w
    for line in f:
        parsed = re.search('(\d{2}:\d{2}:\d{2}.\d{3}) \-\-> (\d{2}:\d{2}:\d{2}.\d{3})', line)
        if parsed:
            start = datetime.timedelta(minutes=int(parsed.group(1)[3:5]), \
                                       seconds=int(parsed.group(1)[6:8]), \
                                       milliseconds=int(parsed.group(1)[9:12]))
            end = datetime.timedelta(minutes=int(parsed.group(2)[3:5]), \
                                       seconds=int(parsed.group(2)[6:8]), \
                                       milliseconds=int(parsed.group(2)[9:12]))
            start *= offset
            end *= offset
            newline = '0' + str(start)[:11] + " --> " + '0' + str(end)[:11] + '\n'
            new.write(newline)

        else:
            new.write(line)
    stop = time.time() - t
    print('The %s file has been updated. New .srt file named %s has been created. The process took %s seconds.' % (n, 'new' + n, stop))

At the top of your file you put all of your imports in one line, but from PEP8:

  Imports should usually be on separate lines

You define offset, but the next line you define it again.  That makes the first definition completely useless.
timeChangeSRT() should not be splitting up a string to get the right minutes, seconds, and milliseconds.  Those should be passed as arguments.
Those backslashes at the end of the lines are useless.  The indentation tells the reader, and the unclosed parentheses tells the interpreter.
You have two lines that are used both in the if and in the else blocks (the definition of newline and writing it to new).  That is a sign that they should be outside of the blocks and executed just afterwards.
Instead of using newline = '0' + str(start)[:11] + " --> " + '0'..., use a format string: newline = '0{} --> 0{}\n'.format(str(start)[:11], str(end)[:11])
You use old-style string formatting, but from the docs on .format() from Python2:

  This method of string formatting is the new standard in Python 3, and should be preferred to the % formatting described in String Formatting Operations in new code.

Your second function pretty much needs the same modifications as the first.  Your second function, in fact, is so similar to the first that I am sure you could define some helper functions to do some of the work so as to reduce the duplicate code.


List of Ordered pairs from unordered Pair in Java Problem statement is

  Write a method that can take an unordered list of airport pairs visited during a trip and return the list in order.

{"ITO", "KOA"}, {"SFO", "SEA"}, {"LGA", "CDG"}, {"KOA", "LGA"}, {"PDX", "ITO"}, {"SEA", "PDX"}

  should return:

(SFO,SEA)(SEA,PDX)(PDX,ITO)(ITO,KOA)(KOA,LGA)(LGA,CDG)

I have come up with this O(n^2) algorithm. I wanted to know if it can be done better than this:

private static void sortPairs(String[][] input)
    {

        for (int i = 0; i < input.length; i++) {
            int j = 1;
            for (; j < input.length; j++) {
                if (input[i][0].equals(input[j][1])) {
                    break;
                }
            }
            if (j == input.length) {
                swapPairs(i, 0, input);
                break;
            }
        }

        for (int i = 0; i < input.length; i++) {
            int j = 1;
            for (; j < input.length; j++) {
                if (input[i][1].equals(input[j][0])) {
                    break;
                }
            }
            if (j == input.length) {
                swapPairs(i, input.length - 1, input);
                break;
            }
        }

        for (int i = 2; i < input.length - 1; i++) {
            int j = i - 1;
            for (; j >= 0; j--) {
                if (input[j + 1][0].equals(input[j][1])) {
                    break;
                }
                swapPairs(j, j + 1, input);
                if (input[j + 1][0].equals(input[j][1])) {
                    break;
                }
            }

        }
    }

    private static void swapPairs(int i, int j, String[][] pairs)
    {
        String[] temp = pairs[i];
        pairs[i] = pairs[j];
        pairs[j] = temp;
    }

Welcome to corereview.se and thanks for sharing your code.

general coding

avoid odd ball solutions

In your inner and outer loop, you use different methods to declare the loop counter variable.
The better way would have bee to find a solution that does not need this odd cod style.

But since you decided to do so you should have placed a comment why you had to do this.

Same is with opening braces {: In your method signature, you place them at a new line, in the rest of the code you put them on the same line as the preceding statement.

THere is a hot discussion out there where to put these opening braces.
I don't want to contribute to that argument,
but all agree that you should decide on either one place and stick to that throughout your code.

business approach

The code does basically three things:

find start point
connect elements
reorder the input array

You chose a procedural approach to the problem. This is not bad by itself, but since Java is an object oriented language you should start looking for OO approaches.

But OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

information hiding / encapsulation
single responsibility
separation of concerns
KISS (Keep it simple (and) stupid.)
DRY (Don't repeat yourself.)
"Tell! Don't ask."
Law of demeter ("Don't talk to strangers!")

A more OOish approach could look like this:

private static void sortPairs(String[][] input) {
    Map<String,String> routes = convertToMap(input);
    String start =  findFirstStart(routes);
    reorderArray(routes, start, input);
}
private static Map<String,String> convertToMap(
         String[][] input) {
    Map<String,String> routes = new HashMap();
    for(String[] route : input)
       routes.put(route[0][1]);
    return routes;
}
private static String findFirstStart(
        Map<String,String> routes) {
     return routes.keySet()
          .stream()
          .filter(city->!routes.values().contains(city))
          .findAny()
          .get();
}
private static void reorderArray(
        Map<String,String> routes,
        String start,
        String[][] input) {
     for(int i = 0 ; i < input.length) {
         String end = routes.get(start);
         input[i][0]=start;
         input[i][1]=end;
         start = end;
     }
}


Find combinations of items in a nested collection It's a very typical problem that I come across here and there. Imagine that you have a collection of items of any type, and sometimes items are collections themselves; you then need to "expand" them and thus multiply the number of items in the result by number of items in that collection. An example would explain it better; consider input/output pairs:

[] -> [[]]
[1] -> [[1]]
[1 2 3 4 5] -> [[1 2 3 4 5]]
[[1 2] 3 4 5] -> [[1 3 4 5] [2 3 4 5]]
[[1 2] 3 [4 5]] -> [[1 3 4] [2 3 4] [1 3 5] [2 3 5]]

so, an entirely one-dimensional collection resolves into itself; a collection where some items are collections of M, N, O items result in a collection of order M×N×O.

The code I've come up with so far is this:

(defn smash [xs ys]
  (if-not (coll? ys)
    (map conj xs (repeat (count xs) ys))
    (mapcat #(smash xs %) ys)))

(defn extrusion [input]
  (loop [result [[]]
         xs input]
    (if (empty? xs)
      result
      (recur (smash result (first xs))
             (rest xs)))))

It gives me the result I need:

user=> (extrusion [])
[[]]
user=> (extrusion [1])
([1])
user=> (extrusion [1 2 3 4 5])
([1 2 3 4 5])
user=> (extrusion [[1 2] 3 4 5])
([1 3 4 5] [2 3 4 5])
user=> (extrusion [[1 2] 3 [4 5]])
([1 3 4] [2 3 4] [1 3 5] [2 3 5])

as well as it works for any number of layers.

Now, I have a very strong sensation that there is a way better solution; perhaps a function from clojure.combinatorics that I'm missing. I'm not talking about time-efficiency at this point, it's purely about lines of code.

If there's a shorter way to express this solution, I'd be super happy to know it. Thanks!

I'm going to admit right off the bat that I don't fully understand what this algorithm is supposed to do. I can make suggestions for extrusion though.

Note how in extrusion you're explicitly calling first and rest on xs once per iteration, then processing the head, and recursing on the tail. The more idiomatic way to split the head from the tail would be to use deconstruction. This also lets you get rid of the call to empty?:

(defn extrusion2 [input]
  (loop [result [[]]
         [x & xs] input] ; Split the "head" from the "tail"

    (if x ; x will be nil when the collection is empty
      (recur (smash result x) xs)
      result)))

I wouldn't use loop here at all though (even though I love loop). You're just iterating over a collection while maintaining an accumulator. This is a textbook case for reduce:

(defn extrusion3 [input]
  (reduce (fn [result x]
            (smash result x))
          [[]]
          input))

Since you conveniently gave smash the same argument order as the required reducing function though, this can be simplified to:

(defn extrusion4 [input]
  (reduce smash [[]] input))

If you really wanted to ridiculous, since input is the last argument to reduce, you can just make the function a partial application of reduce:

(def extrusion5
  (partial reduce smash [[]]))


Making a slideshow in JavaScript/jQuery I am new to JavaScript and jQuery, and I would like to know what can be improved of the code in this slideshow.

(function ($) {
let pointer = 0;

let slides = document.getElementsByClassName('mySlide');
for (let i = 0; i < slides.length; i++) {
    slides[i].style.left = 100 * i + "%";
    $('#dots').append('');
}

function changeSlide() {
    $('#slider').css('left', -100 * pointer + '%');
    changeDot();
    pointer++;
    if (pointer > (slides.length - 1)) pointer = 0;
    setTimeout(changeSlide, 3000);
}

$(document).ready(function () {
    changeSlide();
});

})(jQuery);

As can be observed in the code, I'm changing the left property of the
 inside another
 with overflow: hidden where the images are set to position: absolute.

Cache DOM lookups

When iterating over the elements with class name mySlide in the for loop, there is a look-up for the dots container (i.e. $('#dots')). Bearing in mind that the example only contains four slide elements, that could lead to excess DOM queries. It is good to store references like that and then utilize the reference:

const dotsContainer = $('#dots');

//later: (e.g. in for loop)
dotsContainer.append('');

Consistency

Why mix native JavaScript (e.g. getting the elements with class name mySlide: document.getElementsByClassName('mySlide');) and then use jQuery later on (e.g. getting elements with class name dot: $('.dot')?

That first block to loop over the elements with class name mySlide could be re-written in jQuery style:

const dotsContainer = $('#dots'); //cache the DOM lookup for later
let slides = $('.mySlide');//document.getElementsByClassName('mySlide');
slides.each(function(i, element) { //for (let i = 0; i < slides.length; i++) {
    $(element).css({left: 100 * i + "%"});//slides[i].style.left = 100 * i + "%";
    dotsContainer.append('');
});

Closure simplification

Some of the closures can be simplified - for instance:

$('.dot').each(function () {
  $(this).removeClass('is-active');
});

can be simplified to:

$('.dot').removeClass('is-active');

And the same is true for the DOM-ready callback:

$(document).ready(function () {
    changeSlide();
});

Can be simplified to:

$(document).ready(changeSlide);

jQuery DOM-ready syntax:

As of jQuery 3.0, only $(handler) is recommended; the other syntaxes still work but are deprecated.1

So that ready callback registration can be simplified to:

$(changeSlide);

Many of these tips are explained in more detail in this article from a couple years avo. It does start off by making one question how much jquery is really needed these days- a valid question. For more on that topic, check out you might not need jquery.

(function ($) {

  let pointer = 0;
  //store instead of looking up each time
 const dotsContainer = $('#dots');
  let slides = $('.mySlide');//document.getElementsByClassName('mySlide');
  slides.each(function(i, element) { //for (let i = 0; i < slides.length; i++) {
    $(element).css({left: 100 * i + "%"});//slides[i].style.left = 100 * i + "%";
    dotsContainer.append('');
  });

  $('.dot').each(function (i) {
    $(this).on('click', function () {
      $('#slider').css('left', -100 * i + '%');
      pointer = i;
      changeDot();
    });
  });

  function changeDot() {
    $('.dot').removeClass('is-active');
    $('.dot').eq(pointer).addClass('is-active');
  }

  function changeSlide() {
    $('#slider').css('left', -100 * pointer + '%');
    changeDot();
    pointer++;
    if (pointer > (slides.length - 1)) pointer = 0;
    setTimeout(changeSlide, 3000);
  }

  $(changeSlide);

})(jQuery);
body {
  background: #28262b;
  color: white;
  font-family: 'Roboto', serif-sans;
}

#slider-wrap {
  width: 700px;
  height: 500px;
  background: #39373a;
  border: 1px solid #17151c;
  text-align: center;
  margin: 0 auto;
  overflow: hidden;
}

img {
  width: 100%;
  height: 100%;
}

#slider {
  height: 500px;
  left: 0;
  position: relative;
  transition: all 300ms ease-in-out;
}

.mySlide {
  position: absolute;
  text-align: center;
  z-index: 100;
  top: 0;
  left: 0;
  height: 500px;
}

.photo {
  position: relative;
  width: 700px;
  height: 100%;
}

.description {
  position: absolute;
  bottom: 0;
  left: 0;
  text-align: center;
  padding: 20px 0;
  width: 100%;
  height: 100px;
  background: rgba(0,0,0,.7);
}

#dots {
  display: flex;
  align-items: center;
  justify-content: center;
  margin: 20px;
}
.dot {
  display: block;
  border: 2px solid white;
  border-radius: 50%;
  margin: 0 10px;
  width: 15px;
  height: 15px;
}
.dot:hover {
  cursor: pointer;
  background: #616161;
}

.is-active {
  background: white;
}
.dot.is-active:hover {
  background: pink;
  border-color: pink;
}

        test1

        test2

        test3

        test4

1http://api.jquery.com/ready/


Show furnitures in a different way considering django principle I want in my homepage to show the furnitures items in a following way:

  Newly Arrived Furnitures(upto 10)

  Featured Furnitures

  Free shipping Furnitures (upto 10)

For this I tried to go with the principle of Fat Models, Thin View. Here is my code

class FurnitureQuerySet(models.QuerySet):
    def active(self):
        return self.filter(is_active=True)

    def featured(self):
        return self.active().filter(is_featured=True)

    def new_arrival(self, max_furnitures):
        return self.active().order_by('-id')[:max_furnitures]

    def free_shipping(self, max_furnitures):
        return self.active().filter(is_free_shipping=True)[:max_furnitures]

class FurnitureManager(models.Manager):
    def get_queryset(self):
        return FurnitureQuerySet(self.model, using=self._db)

    def all(self):
        return self.get_queryset().active()

    def featured(self):
        return self.get_queryset().featured()

    def new_arrival(self, max_furnitures):
        return self.get_queryset().new_arrival(max_furnitures)

    def free_shipping(self, max_furnitures):
        return self.get_queryset().free_shipping(max_furnitures)

    def get_by_id(self, slug):
        qs = self.get_queryset().filter(slug=slug)
        if qs.count() == 1:
            return qs.first()
        return None

class Furniture(models.Model):
    name = models.CharField(max_length=100, blank=True, null=True)
    manufacturer = models.ForeignKey(Manufacturer, blank=True, null=True)
    slug = models.SlugField(max_length=200, unique=True)
    is_active = models.BooleanField(
    default=True, help_text='Furniture is available for listing and sale')
    is_featured = models.BooleanField(
        default=False, help_text='Promote this furniture on main pages')
    is_free_shipping = models.BooleanField(
        default=False, help_text='No shipping charges')
    timestamp       = models.DateTimeField(auto_now_add=True)
    objects         = FurnitureManager()

def home(request):
    categories = Category.objects.root_nodes()
    furnitures = Furniture.objects.new_arrival(10)
    featured_furnitures = Furniture.objects.featured()
    shipping_free_furnitures = Furniture.objects.free_shipping(10)
    context = {
        'categories': categories,
        'furnitures': furnitures,
        'featured': featured_furnitures,
        'shipping_free': shipping_free_furnitures,
        }
    return render(request, 'home.html', context)

The template will be:

Newly Arrival

{% for new_furniture in furnitures %}
  {{ new_furniture.name }}
{% endfor %}

Featured

{% for featured_furniture in featured %}
  {{ featured_furniture.name }}
{% endfor %}

Do take care to run flake8 over the source, and follow its advice.

Array slicing [:max_furnitures] is idiomatic and nice. But if you have a thousand or a million items, you will prefer to use SQL limit, so you don't retrieve a ton of items from the DB only to discard them. You will get a different execution plan from the backend DB if you expose limit 10 to its query optimizer.

I see four different max_furnitures parameters. Now maybe this is perfectly nice and we prefer to leave it as is. But also consider introducing it as an attribute used by constructor.

Looks good.

Ship it.


Python Multi Choice Quiz with a score to count I am trying to make a multi choice quiz with a score to count. This is the first time I've used Python and I'm finding it difficult to make the code work properly. How can I make the code shorter and work better? It tells me my code has an infinite loop, how is that so?

#Convert the 0 into a number so we can add scores
score = 0
score = int(score)

#Ask user for their name
name = input("What is your name?")
name = name.title()
print("""Hello {}, welcome to Quiz night!
You will be presented with 5 questions.
Enter the appropriate number to answer the question
Good luck!""".format(name))

#Question1
print("""What is the term for ‘Maori’ language?
1. Te Rex
2. Hangi
3. Hongu
4. Te Reo""")

answer1 = "4"
response1 = input("Your answer is:")

if (response1 != answer1):
    print("Sorry, that is incorrect!")
else:
    print("Well done! " + response1 + " is correct!")
    score = score + 1

print("Your current score is " + str(score) + " out of 5")

#Question2
print("""What is the Maori term for ‘tribe’ or ‘mob’?
1. Mihi
2. Iwi
3. Awi
4. Hapu""")

answer2 = "2"
response2 = input("Your answer is:")

if (response2 != answer2):
    print("Sorry, that is incorrect!")
else:
    print("Well done! " + response2 + " is correct!")
    score = score + 1

print("Your current score is " + str(score) + " out of 5")

#Question3
print("""What is the term for the formal welcome, where two individuals press their nose together?
1. Hongi
2. Haka
3. Hangi
4. Huka""")

answer3 = "1"
response3 = input("Your answer is:")

if (response3 != answer3):
    print("Sorry, that is incorrect!")
else:
    print("Well done! " + response3 + " is correct!")
    score = score + 1

print("Your current score is " + str(score) + " out of 5")

#Question4
print("""Who is the ‘demi-god’ or the ‘great creator’ who fished NZ out from the sea?
1. Zeus
2. Hercules
3. Maui
4. Maori""")

answer4 = "3"
response4 = input("Your answer is:")

if (response4 != answer4):
    print("Sorry, that is incorrect!")
else:
    print("Well done! " + response4 + " is correct!")
    score = score + 1

print("Your current score is " + str(score) + " out of 5")

#Question5
print("""What is the name for the traditional Maori method of cooking?
1. Roast
2. Hangi
3. Hongi
4. Bake""")

answer5 = "2"
response5 = input("Your answer is:")

if (response5 != answer5):
    print("Sorry, that is incorrect!")
else:
    print("Well done! " + response5 + " is correct!")
    score = score + 1

print("Your total score is " + str(score) + " out of 5")
print("Thank you for playing {}, goodbye!".format(name))

The biggest gain in readability here is to get rid of the duplication. Currently all your strings are hardcoded and you print each of them separately.

We can make life easier by putting the questions, answers and correct answers into some datastructure we can iterate over. In addition, we can bundle these three things together into one Question class (which is just a collections.namedtuple here, so we can give the fields nice names):

from collections import namedtuple

Question = namedtuple("Question", ["question", "answers", "correct_answer"])

GREETING = """Hello {name}, welcome to Quiz night!
You will be presented with {n} questions.
Enter the appropriate number to answer the question
Good luck!"""

def play_quiz(name, questions):
    score = 0
    for question in questions:
        print(question.question)
        for i, answer in enumerate(question.answers, start=1):
            print(f"{i}. {answer}")

        response = input("Your answer is: ")
        if response != question.correct_answer:
            print("Sorry, that is incorrect!")
        else:
            print(f"Well done! {response} is correct!")
            score += 1
        print(f"Your current score is {score} out of {len(questions)}")

    print(f"Your total score is {score} out of {len(questions)}")
    print(f"Thank you for playing {name}, goodbye!")

if __name__ == "__main__":
    questions = [Question("What is the term for the ‘Maori’ language?",
                          ["Te Rex", "Hangi", "Hongu", "Te Reo"], "4"),
                 Question("What is the Maori term for ‘tribe’ or ‘mob’?",
                          ["Mihi", "Iwi", "Awi", "Hapu"], "2",
                 ...]
    name = input("What is your name? ").title()
    print(GREETING.format(name=name, n=len(questions)))
    play_quiz(name, questions)


NodeJS module to calculate linear motion I am on the process of changing jobs, so I would like to get an idea of what I do wrong in order to improve.

For this, I have created a small node module. It's very simple; it calculates linear motion (distance, velocity, time). I am interested in knowing what I do wrong, whether it's some crucial mistake or something secondary as the way I format my comments.

/**
*   Checks whether two values are valid to be operated with
*   @param {Float} operandA
*   @param {Float} operandB
*   @return {Boolean}
*/
const areValuesValid = (operandA, operandB) => {
    if(isNaN(operandA) || isNaN(operandB)) return false;

    if(operandA === null || operandB === null) return false;

    return true;
}

/**
*   Rounds a value to a max of two decimals
*   @param {Float} val
*   @return {Float}
*/
const round = (val) => {
    return Math.round(val * 100) / 100;
}

/**
*   Calculates the time in relation to the velocity and the distance
*   @param {Float} velocity
*   @param {Float} distance
*   @return {Float}
*/
const calculateTime = (velocity, distance) => {
    if(areValuesValid(velocity, distance) === false)
        return 0;

    if(parseFloat(velocity) === 0)
        return 0;

    return round(distance / velocity);
}

/**
*   Calculates the velocity in relation to the time and the distance
*   @param {Float} time
*   @param {Float} distance
*   @return {Float}
*/
const calculateVelocity = (time, distance) => {
    if(areValuesValid(time, distance) === false)
        return 0;

    if(parseFloat(time) === 0)
        return 0;

    return round(distance / time);
}

/**
*   Calculates the distance in relation to the velocity and the time
*   @param {Float} velocity
*   @param {Float} time
*   @return {Float}
*/
const calculateDistance = (velocity, time) => {
    if(areValuesValid(velocity, time) === false)
        return 0;

    return round(velocity * time);
}

module.exports = { areValuesValid, calculateTime, calculateVelocity, calculateDistance }

If anybody is willing to go the extra mile, I have created a repository with this code, which also includes some tests. Github. I would like to know if I go about the tests the right way, if the module structure makes sense and whatever else is criticizable.

Nice documentation. This code is clear and easy to read. Here are some basic nits:

You should probably follow standard JSDoc conventions. Don't do this:

/**
*
*/

Do this instead:

/**
 *
 */

If you're nitpicky about the @param, Float technically doesn't exist since all JavaScript numbers are floats. Use Number instead.
See this SO post about rounding to 2 decimal places. Using it is completely up to you, there's nothing wrong with the way you're doing it. (This suggestion is slower, but looks cool I guess)

const round = val => {
    return +val.toFixed(2);
};

You know the constraints of your areValuesValid() function, there's no need to === false.

const calculateTime = (velocity, distance) => {
    if (areValuesValid(velocity, distance) && parseFloat(velocity) !== 0) {
        return round(distance / velocity);
    }
    return 0;
}

Also, why the need to do parseFloat(velocity)? If you know all your arguments will be numbers, this is unnecessary. If you don't know the data type of your arguments, you should probably enforce that they are numbers using typeof.

Happy coding!


Remap ls to honor .hidden files I wrote this code in my .bashrc to remap ls to honor .hidden files. I'm pretty new to shell/bash scripting, so I feel like this is not the most performant way to handle this, and it has some limitations. I would appreciate any suggestions regarding performance improvements, limitation reduction, or common best practices for shell/bash scripting (or improving my post since I'm new to the StackExchange network).

# File names in .hidden files are ignored by ls
# NOTES: Not tolerant of empty lines (or lines with only whitespace) in the .hidden file(s)
#    If any argument doesn't begin with a '-', this assumes it's a file argument
#    If you pass multiple folders, it will essentially concatenate the .hidden files and ignore all files matching any of those names (so if you have a file name TEST in two folders, but only have TEST in the .hidden file of one folder, calling ls on both folders won't show either TEST file)
#    It does honor the -a flag
#    I don't know if there's a limit on total number of flags or total command length, but if there is, this may surpass it depending on the number of file names in the .hidden file
ls () {
    DEFAULT_ARGS="--color=auto" # Here, because I want colors (and you can't have a function and an alias with the same name)
    ALL=false
    A=0
    F=0
    I=0
    declare -a ARGS
    declare -a FILES
    declare -a IGNORE
    for a in $@
    do
        if [[ $a =~ ^-(a|[^-]+a) ]]; then # If the -a flag is present
            ALL=true
            break
        fi
        if [[ $a =~ ^[^-].* ]]; then # If the argument doesn't start with '-', add it to the Files array
            FILES[$F]=$a
            ((F++))
        else                 # Otherwise add it to the Args array
            ARGS[$A]=$a
            ((A++))
        fi
    done
    if $ALL; then
        command ls ${DEFAULT_ARGS} ${@}
        return
    else
        if [ ${#FILES[@]} -eq 0 ]; then # If no files/directories are present, explicitly add the implied .
            FILES[$F]="."
        fi
        for f in ${FILES[*]}
        do
            if [ -f "${f}/.hidden" ]; then # Read the .hidden file, if present, and generate the -I flags
                IGNORE[$I]=$(awk '{print "-I " $0}' "${f}/.hidden" | tr "\n" " ")
                ((I++))
            fi
        done
        command ls ${DEFAULT_ARGS} ${ARGS[*]} ${IGNORE[*]} ${FILES[*]}
        return
    fi
}

Note: A .hidden file is a file (named .hidden) containing a list of file names from the same directory that should be treated as if they begin with a '.'

Some suggestions:

Upper case names are by convention reserved for exported variables.
Use More Quotes™, especially around $@
Use arrays for constructing commands
Variables are by default declared in the global scope. It would be better to declare the variables in the function local.
Use getopt to parse options - the default pattern is a bit wonky but it's easier to deal with for complex cases.
You can use readarray to read .hidden into an array in a single command.
The returns are redundant.
in $@, or more correctly in "$@", is redundant. for argument does the same thing.
You don't need semicolons if you put then on the next line like you already do with do.
You shouldn't have to use index variables. For example, both times $F is used you can instead use FILES[${#FILES[@]}]=…
$* and its accomplice ${name[*]} split every argument on $IFS, and is therefore extremely rarely useful. Use $@ and ${name[@]} instead.


Allocating memory for a matrix with a single malloc For an interview, I was asked to write code allocate memory for a rows*cols matrix, using a single malloc(). I want your comments about this code that I wrote:

/*
Allots (in effect) space for a matrix of size rows*cols using a single malloc.
The whole space allotted consists of an array of size 'rows' of float ptrs,
followed by a float array of size  'rows' * 'cols'.
The float ptrs 1,2..,  are assigned to row no. 1, 2,..  in the matrix that follows.
*/

float** allotMatrix(const size_t &rows, const size_t &cols)
{
    // Compute size of allotment = rows * cols floats + rows float ptrs
    size_t ptrsize = rows * sizeof(float*);
    size_t datasize = rows * cols * sizeof(float);
    size_t matsize =  ptrsize + datasize;

    // Get if possible a memory allotment  of this size. We use char* for convenience
    char* cmatrix = (char*) malloc(matsize);
    if (cmatrix == NULL)
    {
        fprintf(stderr, "Unable to allot memory");
        exit(1);
    }

    memset((void*) cmatrix, 0, matsize);

    // Assign ptrs to data
    float** ptrstart = (float**) cmatrix;
    float* datastart = (float*) (cmatrix + ptrsize);
    for (size_t i = 0; i != rows; ++i)
    {

        *ptrstart =  datastart;
        // Move to next ptr
        ++ptrstart;
        // .. and move to the next data row
        datastart += cols;
    }

    return (float**) cmatrix;
}

I would use calloc instead of malloc since you're anyway clearing the matrix after allocation.

void* cmatrix = calloc(1, matsize);
...
//memset((void*) cmatrix, 0, matsize);  this is not needed anymore because of 'calloc'

Also I do not think there is any point in using char* here because. Looks like you're using char* only because it allows you to increment pointers by bytes. I think using float** here would be even more convinient:

float** cmatrix = (float**) calloc(1, matsize);
if (cmatrix == NULL) { ... }
float* datastart = (float*) (cmatrix + rows); // you don't need char pointers here
for (...) { ... }


From enumerator to map I have an mail object that returns its headers as an Enumerator of Header objects, which have two methods, getName and getValue. I need to convert that to a nice Clojure map so I wrote this short function:

(defn extract-headers [message]
  (reduce (fn [hs h] (conj hs {(.getName h) (.getValue h)}))
          {}
          (enumeration-seq (.getAllHeaders message))))

Anything to change or improve here?

You can use into, for example:

(defn extract-headers [message]
  (let [headers (enumeration-seq (.getAllHeaders message))]
       (into {} (map #(vector (.getName %1) (.getValue %1)) headers))))

first, with map you build sequence of 2-element vectors, that later is converted into map


Toggling image sources I have a set of images with class = tt like so:

<img src="images/arc_unselected.png" title="Arc" class="tt" />

and I have written this code to change the images after clicking:

     $(".tt").click(function(event) {
           var title = event.target.getAttribute("title")
           resetOtherImages(title)
           if( title == "info"){
              if ( event.target.getAttribute("src") == "images/information_selected.png"){
                 $(this).attr('src', 'images/information_unselected.png')
                 }
              else {
                  $(this).attr('src', 'images/information_selected.png')
                 }
           }else if( title == "Up"){
                 if ( event.target.getAttribute("src") == "images/up_selected.png"){
                 $(this).attr('src', 'images/up_unselected.png')
                 }
              else {
                  $(this).attr('src', 'images/up_selected.png')
                 }
           }else if( title == "Down"){
                 if ( event.target.getAttribute("src") == "images/down_selected.png"){
                 $(this).attr('src', 'images/down_unselected.png')
                 }
              else {
                  $(this).attr('src', 'images/down_selected.png')
                 }

           }else if( title == "SE"){
                 if ( event.target.getAttribute("src") == "images/se_selected.png"){
                 $(this).attr('src', 'images/se_unselected.png')
                 }
              else {
                  $(this).attr('src', 'images/se_selected.png')
                 }
           }else if( title == "Arc"){
                 if ( event.target.getAttribute("src") == "images/ar_selected.png"){
                 $(this).attr('src', 'images/arc_unselected.png')
                 }
              else {
                  $(this).attr('src', 'images/arc_selected.png')
                 }
           }
        })

        function resetOtherImages(title) {
        $('.tt').each(function(){
            if($(this).attr('src') !== title)
            {
              var newName = changeName($(this).attr('src'))
              $(this).attr('src',newName)
            }
        });
        }

        function changeName(imgName) {
           var pos = imgName.indexOf("_");
           var first = imgName.substring(0,pos);
           return first.concat("","_unselected.png");
        }

It works without any problem, but since I have not much experience with javascript I wanted to know if there is a better way to do this, more compact and more readable.

Interesting question, in that you gave away most of the answer in this function:

    function changeName(imgName) {
       var pos = imgName.indexOf("_");
       var first = imgName.substring(0,pos);
       return first.concat("","_unselected.png");
    }

You know that all unselected images end with "_unselected.png" and by extension you know that all selected images end with "_selected.png". As a side note I would not have used indexOf, and substring, I would have used a split and string concatenation thru +:

    function deriveUnselectedVersion( imagePath ){
       var firstPart = imagePath.split("_")[0];
       return firstPart + '_unselected.png';
    }

I am not 100% confident about deriveUnselectedVersion but it sure sounds better (and is more meaningful) than changeName.

Anyway, this is how I would write the click handler:

 $(".tt").click(function(event) {
       var image = event.target,
           title = image.getAttribute("title"),
           sourceInfo = image.getAttribute("src").split("_"),
           base = sourceInfo[0],
           version = sourceInfo[1];

       resetOtherImages(title);

       if( version == "selected.png" ){
         $(this).attr('src', base + "_unselected.png");
       } else {
         $(this).attr('src', base + "_selected.png");
       }

       //Or we could go really (too) fancy
        $(this).attr('src', base + ( version == "selected.png" ? "_unselected.png" : "_selected.png" ) );

    });

You will notice:

No missing semi colons at the end of statements
1 var statement on top to rule them all
Only 1 $() statement executed (these are expensive)
This will work for all new buttons as long they follow the same naming scheme
Much shorter
I am still ambivalent about using var title, since title is used in only one place..

A final advice is that writing code follows the same rules as writing prose, dont repeat yourself (too much) aka DRY.

    function resetOtherImages(title) {
    $('.tt').each(function(){
        if($(this).attr('src') !== title)
        {
          var newName = changeName($(this).attr('src'))
          $(this).attr('src',newName)
        }
    });
    }

See how many times you repeat $(this).attr('src') ? You can and should do better than that.


Print periphery of a binary tree I want you to pick my code apart and give me some feedback on how I could make it better or more simple. This code will traverse peripheri  (clock and anticlockwise) in \$O(n)\$ just traversing the tree once.

public class PeriPheri {

    private TreeNode root;

    private static class TreeNode {
        TreeNode left;
        TreeNode right;
        int item;

        TreeNode(TreeNode left, TreeNode right, int item) {
            this.left = left;
            this.right = right;
            this.item = item;
        }
    }

    private void printChild (TreeNode node) {
        if (node == null) {
            return;
        }

        if (node.left == null && node.right == null) {
            System.out.print(node.item + " ");
        }

        if (node != null) {
            printChild (node.left);
            printChild (node.right);
        }
    }

    private void leftAntiClock(TreeNode node) {
        if (node != null) {
            System.out.print(node.item +  " ");
            leftAntiClock (node.left);
            printChild (node.right);
        }
    }

    private void rightAntiClock(TreeNode node) {
        if (node != null) {
            printChild (node.left);
            rightAntiClock (node.right);
            System.out.print(node.item + " ");
        }
    }

    public void antiClockwise() {
        if (root == null) {
            return;
        }

        System.out.print (root.item + " ");
        leftAntiClock (root.left);
        rightAntiClock (root.right);
    }

    private void rightClock(TreeNode node) {
        if (node != null) {
            System.out.print(node.item + " ");
            rightClock(node.right);
            printChild(node.left);
        }
    }

    private void leftClock(TreeNode node) {
        if (node != null) {
            printChild(node.right);
            printChild(node.left);
            System.out.print(node.item + " ");
        }
    }

    public void clockwise() {
       if (root == null) {
          return null;
        }
        System.out.print(root.item + " ");
        rightClock (root.right);
        leftClock (root.left);
    }

}

I think that printChild() would be better named printDescendantLeaves().  Also, printChild() always works left-to-right, which spoils your clockwise traversal.

For flexibility, consider taking a visitor instead of hard-coding System.out.println() everywhere.  Alternatively, implement your traversal as an Iterator (which is admittedly tricky to do recursively since Java doesn't support yield).

"Periphery" is misspelled.


Class method to insert a record into MySQL I'm just starting out using OOP; classes and methods etc. I've been looking around for some PDO classes/wrappers out there, but there's not much to choose from. So I've tried to make my own. Started out by writing an "insert" method.

As a man with low self esteem, I never like what I do myself, so I thought I'd ask you guys here for feedback. Here it is:

public function insert($tbl, $data) {

    $this->_stmt = $this->_dbh->prepare("INSERT INTO $tbl (" . implode(', ', array_keys($data)) . ") VALUES (:" . implode(', :', array_keys($data)) . ")");

    foreach($data as $key => $value) {
        $this->_stmt->bindValue($key, $value, PDO::PARAM_STR);
    }

    $this->_stmt->execute();
}

The meaning was that it could all be done in one sweep instead of having several methods to do one thing; inserting a record.

An example of using this code:

$message   = 'A message for the ones who like to read it!';
$sent_on   = 'Saturday 23rd 2012';
$unique_id = 'unique_as_can_get';

$data = array(
    'message'   => $message,
    'sent_on'   => $sent_on,
    'unique_id' => $unique_id
);

$insert = new DB;
$insert->insert('tablename', $data);

I've put the database connection into the constructor.

The code works as I want it to, but I'm still confused if it's "accepted" amongst you people who are way more skilled that I will ever be.

Please let me know if this code if "usable" and/or what is wrong with it, what can be improved/changed etc.

There's nothing wrong with it, but it's a small piece of code. Some minor details are:

1: I would write $table instead of $tbl, why abbreviate it? You also write foreach ($data as $key => $value), which is very general. Why not specify it better: `foreach ($row as $column => $value)'? What I mean is that variable names should have meaning. I know an array has keys and values, but they are general names. Here you should use names that tell you what a variable really represents.

2: $this->_stmt is a class variable, where a local $statement variable would do. Local variables are always more efficient and have even better encapsulation.

3: Also watch your line length: 156 characters is too much. Instead of writing this:

$this->_stmt = $this->_dbh->prepare("INSERT INTO $tbl (" . implode(', ',
array_keys($data)) . ") VALUES (:" .  implode(', :', array_keys($data)) . ")");

(I wrapped it for clarity), you could have written something like;

$rowkeys   = array_keys($row);
$columns   = implode(',',$rowkeys);
$values    = ':'.implode(',:',$rowkeys);
$query     = "INSERT INTO $table ($columns) VALUES($values)";
$statement = $this->handle->prepare($query);

This makes it easier to read, and debug. It may seem longer, but effectively it does the same thing.

4: You could build in error checks. Is the array a valid array to insert? Start with is_array() for instance. Do the column names exist in the table? Does the insert execute properly? No errors?

5: you could return the lastInsertID(): http://php.net/manual/en/pdo.lastinsertid.php

6: You cannot insert multiple rows at once with this method. Perhaps you don't need this, but if you do you could add that functionallity.

7: You bind values, so that's quite secure. However, are you sure your column names can never be influenced by outside sources (= hackers)? Another good reason to check them, because they go straight into your SQL command.

8: Please note that wrapping PDO can be a burden later. See: Class for reducing development time If that puts you complete off, don't worry, I also use a wrapper despite all that good advice.


Handling messages from a server I've never really done anything with concurrency, so I'm not sure if this is done correctly.

Basically, I have a class that handles all messages from the server and decides what to do them. It uses a separate thread to keep looping through a list to check for any new messages it has to deal with.

The main server thread needs to be able to add messages to the ArrayList without causing a concurrency error, and so far it seems ok but I don't have that many messages coming in.

public class MessageThread implements Runnable {
    public static ArrayList<Message> messages = new ArrayList<Message>();

    @Override
    public void run() {
        while (true) {
            synchronized (messages) {

                for (Message m : messages) {

        }
    }
}
    public static void addMessage(Message message) {
        synchronized (messages) {
            messages.add(message);
        }
    }

    public static void removeMessage(Message message) {
        synchronized (messages) {
            messages.remove(message);
        }
    }
}

So as you can see, the server thread would use the MessageThread.addMessage method whenever it needs to add a message.

Your ArrayList is correctly guarded from access by multiple threads but you've got another problem. The busy wait in your run method is going to hog the CPU and might even starve the other threads that would be trying to put a message in the queue. You'll need some way of signaling that the queue is non-empty so that the run function can wait on the signal. In .NET it would be a ManualResetEvent. I don't know the java equivalent.  Or better yet, have a look at BlockingQueue.

@Override
public void run() {
    while (true) {
        queueFullEvent.waitOne();
        synchronized (messages) {
            for (Message m : messages) {
                // messages should be removed here
                messages.remove(m);
                if (messages.isEmpty)
                    queueFullEvent.reset();
            }
        }
    }
}

public static void addMessage(Message message) {
    synchronized (messages) {
        messages.add(message);
        queueFullEvent.set();
    }
}

public static void removeMessage(Message message) {
   // would require additional syncronization objects to avoid a race.
}

}


Abstracting out two lines of code in Coffeescript I am currently working on refactoring two methods that exist in directives into a service.

This is method one:

scope.computeStyle = (component) ->
  elementHash = {}
  if component.element.type == 'table'
    elementHash.height = 600
    if component.height?
      elementHash.height = component.height * 50
  else if component.element.type == 'single_value'
    elementHash.height = 200
    elementHash.width = 800
  return elementHash

This is method two:

scope.computeStyle = (element, rowComponent) ->
  elementHash = {}
  if element.type == 'table'
    elementHash.height = 600
    if rowComponent?.height?
      elementHash.height = rowComponent.height
  else if element.type == 'single_value'
    elementHash.height = 200
    elementHash.width = 800
  return elementHash

They both do the same thing (a little differently). They take an element/component (which are objects containing data about an object in my application), checks what type of object they are (in this case, a table or a single value), and applies a height and/or width transformation.

Within my service, this is what I have done:

angular = require "angular"

angular.module("myapp.dashboards.layouts").service("DashboardLayoutComputeStyle" ->

  @computeStyle = (componentElement, height) ->
    elementHash = {}
    if componentElement.type == 'table'
      elementHash.height = 600
      if height?
        elementHash.height = height
    else if element.type == 'single_value'
      elementHash.height = 200
      elementHash.width = 800
    return elementHash

  @
)

Here's where my question comes in:

I am not sure what to do at this line in my abstracted method:

  if height?
    elementHash.height = height

In method one, this is what it looks like:

    if component.height?
      elementHash.height = component.height * 50

In method two, this is what it looks like:

    if rowComponent?.height?
      elementHash.height = rowComponent.height

I have three constraints to the refactoring of these two lines:

I must check whether both rowComponent and height exist in method two
I don't need to check whether component exists, but I must check whether height exists in method one
In method one, I must set the elementHash,height to component.height * 50, while in method two, I can simply assign rowComponent.height to elementHash.height

How would I abstract these two lines out?

I'd say your abstracted function looks OK - it's up to the caller to supply the right height. That's the abstraction. The function doesn't have to know exactly what it's looking at, it just needs an element and (optionally) a height.

So you either call:

@computeStyle componentA.element, componentA.height * 50

or

@computeStyle someElement, componentB?.height

if I'm not mistaken. Something like that anyway.

I'd be more worried about the magic width/height values in the function, to be honest. And I'd encourage you to use is rather than == - both compile into === so there's zero difference, but I'd use the more coffeescript'y is.

Oh, and you have an error in your else if conditional: You're referencing element, but in your abstracted function, it's called componentElement

Incidentally, since returns are implicit at the exit point of a function, and object literals can be written quite tersely, you can reduce your function to this:

computeStyle = (componentElement, height) ->
  switch componentElement.type
    when 'table'
      height: height or 600
    when 'single_value'
      height: 200
      width: 800
    else
      {}

or even more tersely:

computeStyle = (componentElement, height) ->
  switch componentElement.type
    when 'table' then height: height or 600
    when 'single_value' then height: 200, width: 800
    else {}


Dynamic Multithreading lock I need to create an structure to control dynamically locks. For example, I have 100 threads running, and each thread will "process" a "key". Sometimes two threads could be processing the same "key", and if it occurs, a dynamic lock needs to be created and used by both, to process one at a time.

I create the following structure and I would like a code review, to find possible bugs (even if it is extremely rare):

  // This is the class who control to create one different lock for each key.
    public class DynamicLockGenerator
{
    private static Dictionary<string, object> _locksByKeys = new Dictionary<string, object>();

    private static object _lock = new object();

    public static objectGetLock(string key)
    {
        lock (_lock)
        {
            if (!_locksByKeys.ContainsKey(key))
                _locksByKeys.Add(key, new object());

            return _locksByKeys[key];
        }
    }

    public static void RemoveLock(string key)
    {
        lock (_lock)
        {
            //This line is extremely important: One lock object can only be removed
            //if no thread are using it.
            if (Monitor.TryEnter(_locksByKeys[key]))
            {
                _locksByKeys.Remove(key);
            }
        }
    }
}

//This is the use of the class. All threads do the same thing, but with different (or equal) keys.
object lockObj = DynamicLockGenerator.GetLock(key);

lock (lockObj)
{
    //PROCESS THE KEY
}

DynamicLockGenerator.RemoveLock(key);

I'm pretty sure you have the potential for a race condition if one thread releases the lock and removes it, after another thread has got the lock but not yet locked it.

Thread one GetLock(someKey) // Creates lock, returns dynamic key
Thread one Lock(dynamicKey)
Thread one complete processing (exits lock section)
Thread two GetLock(someKey) // returns existing key
Thread one RemoveLock // removes lock from collection (not yet being used)
Thread two Lock(dynamicKey) // Now we've locked an object no longer in collection
Thread two complete processing (exit lock block)
Thread two Remove lock // tries to remove a lock that's not in the collection


Getting the Sum of Discounted Price Jojo is browsing the internet while suddenly he sees an ad about the new cafe. The promotion is if the price
of an item is N dollars, then you can buy the second item for half the price, the third item for a quarter of the
original price, and so on, but if it becomes less than M dollars, then you have to pay M dollars. He wonders
how much he has to pay if he buys K item.

Format Input

The first line will contain an integer T, the number of test cases.
Each test case will have 3 integers N, M, and K, each denoting the original price, the minimum price, and the
amount Jojo is going to buy.

Format Output

For each test case, print “Case #X: “ (X starts with 1), then followed by the price Jojo has to pay rounded to 3
decimal digits.

Constraints

1 <= T <= 10

1 <= M <= N <= 1,000,000,000

1 <= K <= 1,000,000,000

#include <stdio.h>

    int main()
    {
        int T;
        scanf("%d", &T);

        for (int i = 1; i <= T; i++) {
            int M, K, N;
            scanf("%d %d %d", &N, &M, &K);

            float halfPrice = N;
            float sum = halfPrice;

            for (int j = 0; j < K-1; j++) {
                halfPrice /= 2;
                halfPrice = (halfPrice < M) ? M : halfPrice;
                sum += halfPrice;
            }
            printf("Case #%d: %.3f\n", i, sum);
        }
    }

Formatting
This may be a copy and paste error, but generally int main() will start in the first column like the #include does. Other the rest of the indentation needs to be based on that. This was correctly done in your earlier question.

#include <stdio.h>

int main()
{
    int T;
    scanf("%d", &T);

    ...

}

Functions
You may not have run into difficult problems yet, but as the programs get more complex the best way to solve them will be to break it up into smaller pieces where each particular task is easy to solve. In this case the outer for loop would make a good function. Perhaps the inner loop would make a second good function.

Variable Names
Variable names should be meaningful to make the code more readable, easier to understand and maintainable.

The problem statement gives you the variable names T, N, M and K which are currently being used in the code, however, it also gives you what the variables are used for. It might be better to use names such as testCount for T, maxPrice for N, minPrice for M and itemCount for K.

The variable name halfPrice could be misleading be it won't always be half of the maximum price.

Mixing Types in Comparisons
There is the possibility of Floating Point Error in the code because of this comparison:

            halfPrice = (halfPrice < M) ? M : halfPrice;

While the minimum price is input as an integer it might be better to convert it to a float and use that in the comparison. This conversion should only be done once per test case.

In many cases banks work around this by keeping amounts in 2 integer values, one for dollars and one for cents (units are USA, units may differ based on location).


Image to HTML / CSS conversion using pure JS Just for fun I wrote some JavaScript that "converts" an image into pure HTML / CSS. The user is able to select a local image. This image is loaded into an invisible canvas in order to extract pixeldata from the image. Using the pixeldata, a structure of 1x1 <div/> elements is created with each element having its background color set to its corresponding pixel color of the image.

The script works perfectly fine on small (width / height) images, but kills the browser on any large image file. I was wondering if there are some things I can do within my code to solve this issue?

document.addEventListener("DOMContentLoaded", function(event) {
 const file = document.getElementById('file');

 file.addEventListener('change', handleFile);

 function handleFile(e){
  // Create canvas
  let canvas = document.createElement('canvas');
  let ctx = canvas.getContext('2d');

  // Load image
  let img = new Image;
  img.src = URL.createObjectURL(e.target.files[0]);
  img.onload = function() {
   // Draw image to canvas
   ctx.drawImage(img, 0, 0, img.width, img.height);

   let container = document.createElement('div');
   container.id = 'container';
   container.style.Width = img.width;
   container.style.Height = img.height;

   let pixelData = ctx.getImageData(0, 0, img.width, img.height).data;
   let pixel = 0;

   // Loop through each row
   for(let x = 0; x < img.height; x++){
    let row = document.createElement('div');
    row.className = 'row';

    // Loop through each column
    for(let y = 0; y < img.width; y++){
     let col = document.createElement('div');
     col.className = 'col';

     col.style.cssText = 'background: rgba('+pixelData[pixel]+','+pixelData[pixel+1]+','+pixelData[pixel+2]+','+pixelData[pixel+3]+');';
     row.appendChild(col);
        pixel = pixel + 4;
    }
    container.appendChild(row);
   }

   document.getElementById('body').appendChild(container);

   URL.revokeObjectURL(img.src);
  }
 }
});
#container {
 margin-left: 50px;
 margin-top: 50px;
}
.row {
 overflow: auto;
}
.row {
 height: 1px;
}
.col {
 width: 1px;
 height: 1px;
 float:left;
}
<body id='body'>
 <input type='file' id='file'/>
</body>

After working on this for a while with other developers on Stack Overflow chat, the best solution I came up with (so far) was to use an asynchronous loop and adding nodes to a Document Fragment instead of adding them directly to the DOM.

The result:

document.addEventListener("DOMContentLoaded", function(event) {
 const file = document.getElementById('file');

 file.addEventListener('change', handleFile);

 function handleFile(e){
  document.getElementById('container').innerHTML = '';

  // Load image
  let img = new Image;
  img.src = URL.createObjectURL(e.target.files[0]);
  img.onload = function() {
   // Create canvas
   let canvas = document.createElement('canvas');
   let ctx = canvas.getContext('2d');
   canvas.width = img.width;
   canvas.height = img.height;

   // Draw image to canvas
   ctx.drawImage(img, 0, 0, img.width, img.height);

   let container = document.getElementById('container');
   container.style.width = img.width+'px';
   container.style.height = img.height+'px';

   let pixelData = ctx.getImageData(0, 0, img.width, img.height).data;
   let pixel = 0;
   let processedPixels = 0;
   let total = img.height*img.width;

   (function _asyncLoop(){
    let row = document.createElement('div');
    let fragment = document.createDocumentFragment();
    row.className = 'row';

    let rowPixels = 0;

    do {
     let col = document.createElement('div');
     col.className = 'col';

     col.style.cssText = 'background: rgba('+pixelData[pixel]+','+pixelData[pixel+1]+','+pixelData[pixel+2]+','+pixelData[pixel+3]+');';
     fragment.appendChild(col);
     processedPixels++;
     rowPixels++;
     pixel = pixel + 4;
    } while(rowPixels < img.width);

    row.appendChild(fragment);
    document.getElementById('container').appendChild(row);

    if( processedPixels < total ) setTimeout( _asyncLoop, 100 );
   }());

   URL.revokeObjectURL(img.src);
  }
 }
});
#container {
 margin-left: 50px;
 margin-top: 50px;
}
.row {
 overflow: auto;
}
.row {
 height: 1px;
}
.col {
 width: 1px;
 height: 1px;
 float:left;
}

Select an image from your computer and allow Javascript to draw it in the DOM using only DIV elements!

 <input type='file' id='file'/>


Checking locked users using Identity 2.0 I'm writing a theoretical MVC application to aid in learning more about ASP Identity 2. In real terms I'm new to ASP Identity as a whole but thought I'd jump in in this release as

it's the default in new projects and
everything I read about it seems to mostly suit my needs.

There is one issue that I need to be able to overcome. That issue is locking out users. I want to allow administrators the facility to be able to lock out a user should they wish to restrict their access.

From what I've read so far, there is a temporary lockout solution for example when the user tries their password incorrectly x number of times for y number of minutes. This doesn't seem to be the recommended method for more long term solutions.

For a longer term solution, I've added a Locked property to the ApplicationUser class in Entity Framework code first:

public class ApplicationUser : IdentityUser
{
    public string FirstName { get; set; }
    public string Surname { get; set; }
    public bool Locked { get; set; }
    [NotMapped]
    public string FullName
    {
        get
        {
            return FirstName + " " + Surname;
        }
    }
}

Getting to the point of my question though, which is, is my modified ActionResult Login method secure and efficient? I don't want to make unnecessary roundtrips to the database and I also don't want to unwillingly do anything unsecure.

public async Task<ActionResult> Login(LoginViewModel model, string returnUrl)
    {
        if (!ModelState.IsValid)
        {
            return View(model);
        }
        // MY LOCKED CHECK CODE:
        // Check if the user is locked out first
        // 1. Fail if it is
        // 2. If user isn't found then return invalid login atempt
        // 3. If the user is found and it isn't locked out then proceed with the login as usual
        var user = UserManager.Users.Where(u => u.UserName == model.Email).SingleOrDefault();
        if (user != null)
        {
            if (user.Locked)
            {
                return View("Lockout");
            }
        }
        else
        {
            ModelState.AddModelError("", "Invalid login attempt.");
            return View(model);
        }
        // END MY CODE

        // This doesn't count login failures towards account lockout
        // To enable password failures to trigger account lockout, change to shouldLockout: true
        var result = await SignInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, shouldLockout: false);
        switch (result)
        {
            case SignInStatus.Success:
                return RedirectToLocal(returnUrl);
            case SignInStatus.LockedOut:
                return View("Lockout");
            case SignInStatus.RequiresVerification:
                return RedirectToAction("SendCode", new { ReturnUrl = returnUrl, RememberMe = model.RememberMe });
            case SignInStatus.Failure:
            default:
                ModelState.AddModelError("", "Invalid login attempt.");
                return View(model);
        }
    }

Only focusing on this part

var user = UserManager.Users.Where(u => u.UserName == model.Email).SingleOrDefault();
if (user != null)
{
    if (user.Locked)
    {
        return View("Lockout");
    }
}
else
{
    ModelState.AddModelError("", "Invalid login attempt.");
    return View(model);
}

Are you aware that if the Users contains more than one user with the same Email the call to SingleOrDefault will throw an InvalidOperationException ? If you are 100 percent sure that this won't ever happen then there won't be a problem.

In its current state this linq query will for each user query the model for the Email property. If you store it in a variable the access will be much faster.

Checking first if the user == null will make it more obvious what is happening. As I first looked over this code I thought hey, how should the code after the if..else ever be reached.

Applying this will lead to

    string email = model.Email;
    var user = UserManager.Users.Where(u => u.UserName == email).SingleOrDefault();
    if (user == null)
    {
        ModelState.AddModelError("", "Invalid login attempt.");
        return View(model);
    }

    if (user.Locked)
    {
        return View("Lockout");
    }

This looks far better IMO and is more readable.

Usually a username isn't checked in a case sensitive way so instead of u.UserName == email you should use the string.equals() method.

This method takes as the third parameter a StringComparison enum which defines how the comparison will take place. The best one for all cultures, for instance using a turkish locale can make problems (does-your-code-pass-turkey-test), will be to use StringComparison.OrdinalIgnoreCase.

Applying this will lead to

    string email = model.Email;
    var user = UserManager.Users.Where(u => string.Equals(u.UserName, email, StringComparison.OrdinalIgnoreCase)).SingleOrDefault();
    if (user == null)
    {
        ModelState.AddModelError("", "Invalid login attempt.");
        return View(model);
    }

    if (user.Locked)
    {
        return View("Lockout");
    }


Node.js DynamoDB callback I am writing an API which gets results from a DynamoDB table and puts the JSON back into the browser.

The code below works and returns the desired results.  However, after reading about async and callbacks, it's becoming important for me to know if I should be writing this in another way.  Does the following scale well with hundreds of concurrent API callers?

This code does not seem to be using callbacks.  Is it asynchronous?

var restify = require('restify');
var AWS = require('aws-sdk');
AWS.config.update({region: 'us-east-1'});
var db = new AWS.DynamoDB();

function myFunction(req, res, next) {
  var params = {
    TableName : 'myTable',
    KeyConditions :
      {
        "number" :
          {
            "AttributeValueList" : [
              {
                "S" : req.params.simethingid
              }
            ],
            "ComparisonOperator" : "EQ"
          }
      }
  }

  db.query(params, function(err, data) {
    if (err) {
      console.log(err);
      }
    else {
      res.statusCode = 200;
      res.setHeader('Content-Type', 'text/plain; charset=UTF-8');
      res.send(JSON.stringify(data, undefined, 2));
      console.log(JSON.stringify(data, undefined, 2))
      res.end();
      }
  });
  return next();
 }

server.get('/searchme/:somethingid', myFunction);

Can't speak to how it'll scale, but for your 2nd question: Yes, the code above is asynchronous.

When you call db.query() or server.get() you pass a callback as the 2nd argument. That's the asynchronous stuff, as you don't know when that callback will be invoked.

You basically just say "call me when it's done" and continue about your business (i.e. in the case of db.query() the evaluation will proceed to return next(); without "waiting" for the query to finish).

If it were synchronous, it'd be something like result = db.query(params); where everything would wait for the query to return something to assign to result.

By the way (so this is actually a review and not just an explanation)

fix your indentation
in JS it's better to not use the brace-on-new-line style
you're missing some semi-colons
you don't need to quote all those property names in the params object
beware of using undefined, since that isn't actually a keyword. Some runtimes let you say undefined = 23 in which case undefined is then just another (defined) variable. Use null instead


Interpreting database values as times and integers I am working through Chapter 10 of Haskell Programming from First Principles.

I've gotten the code to work, but I am uneasy about the fact that a few of my functions - namely, extractUTCTime and extractInteger - are not total functions. There is valid input for which those functions are bottom. I've added the code below, but I was wondering what strategies are available for making extractUTCTime and extractInteger safe. (One strategy that I do know of is changing the type signature of these functions so that they return a value of type Maybe: extractUTCTime :: DatabaseItem -> Maybe UTCTime. I was wondering if there was anything else that can be done).

import Data.Time

data DatabaseItem = DbString String | DbNumber Integer | DbDate UTCTime deriving (Eq, Ord, Show)

theDatabase :: [DatabaseItem]
theDatabase = [DbDate (UTCTime (fromGregorian 1911 5 1) (secondsToDiffTime 34123)), DbNumber 9001, DbString "Hello, world!", DbDate (UTCTime (fromGregorian 1921 5 1) (secondsToDiffTime 34123))]

filterDbDate :: [DatabaseItem] -> [UTCTime]
filterDbDate [] = []
filterDbDate dataBase = [extractUTCTime item | item <- dataBase, isDbDate item]

isDbDate :: DatabaseItem -> Bool
isDbDate (DbDate _) = True
isDbDate _          = False

extractUTCTime :: DatabaseItem -> UTCTime
extractUTCTime (DbDate utcData) = utcData

filterDbNumber :: [DatabaseItem] -> [Integer]
filterDbNumber [] = []
filterDbNumber dataBase = [extractInteger item | item <- dataBase, isDbNumber item]

extractInteger :: DatabaseItem -> Integer
extractInteger (DbNumber int) = int

isDbNumber :: DatabaseItem -> Bool
isDbNumber (DbNumber _) = True
isDbNumber _ = False

mostRecent :: [DatabaseItem] -> UTCTime
mostRecent dataBase = foldr max y ys
                where dates = filterDbDate dataBase
                      y     = head dates
                      ys    = tail dates

sumDb :: [DatabaseItem] -> Integer
sumDb dataBase = foldr (+) 0 dbNumbers
                 where dbNumbers = filterDbNumber dataBase

avgDb :: [DatabaseItem] -> Double
avgDb dataBase = dividend / divisor
                 where dividend = fromIntegral $ sumDb dataBase
                       divisor = fromIntegral $ length $ filterDbNumber dataBase

I was wondering if there was anything else that can be done

Well, morally, no. You can either return a nullable value (Maybe UTCTime) or lie (read "1970-01-01 00:00:00 UTC" :: UTCTime) but you can't turn a DbNumber 1 :: DatabaseItem into a UTCTime both unconditionally and truthfully.

The good news is, if you hit on the right set of higher-order functions you don't need to. Maybe values are very easy to work with when you know the extra tools available in Data.Maybe.

Taking as a whole this set of functions related to dates—

filterDbDate :: [DatabaseItem] -> [UTCTime]
filterDbDate [] = []
filterDbDate dataBase = [extractUTCTime item | item <- dataBase, isDbDate item]

isDbDate :: DatabaseItem -> Bool
isDbDate (DbDate _) = True
isDbDate _          = False

extractUTCTime :: DatabaseItem -> UTCTime
extractUTCTime (DbDate utcData) = utcData

mostRecent :: [DatabaseItem] -> UTCTime
mostRecent dataBase = foldr max y ys
                where dates = filterDbDate dataBase
                      y     = head dates
                      ys    = tail dates

We'll begin by throwing out isDbDate and extractUTCTime and writing the DatabaseItem -> Maybe UTCTime function you avoided.

dbDate :: DatabaseItem -> Maybe UTCTime
dbDate (DbDate utc) = Just utc
dbDate _            = Nothing

Now pulling from Data.Maybe, we'll replace filterDbDate with an equivalent code fragment that utilizes mapMaybe.

-- filterDbDate :: [DatabaseItem] -> [UTCTime]
-- filterDbDate db =
     mapMaybe dbDate db

We can use that definition in mostRecent, a function from the standard Prelude, and point free style to achieve a one liner that more than just being clever, emphasizes the power of abstraction in producing concise code that will be instantly comprehensible to other Haskell programmers.

mostRecent :: [DatabaseItem] -> UTCTime
mostRecent = maximum . mapMaybe dbDate

mostRecent still commits the sin of using a partial function though, maximum throws an exception (much like your use of head and tail did before) when the filtered list is empty. Until such time as partial functions are excised from base (and O, do we all lust for and fear such a tumultuous day) you should familiarize yourself with the safe package, which has no dependencies besides base, and provides many alternatives to the partial functions we all know and love.

import Safe.Foldable (maximumMay)

mostRecent :: [DatabaseItem] -> Maybe UTCTime
mostRecent = maximumMay . mapMaybe dbDate


Machine epsilon - find epsi 4.2 Machine Epsilon

  Find the floating point number epsi that has
  the the following properties:

  1.0+epsi is greater than 1.0 and
  Let m b e  any number less than epsi. Then 1.0+m is equal to 1.0.

  epsi is called machine epsilon. It is of great importance in understanding floating point numbers.

I have written the following program to try and find my machine epsilon value to a given search depth (10).  Do you have any ideas how I could write this program better?

(defpackage :find-epsi (:use cl))
(in-package :find-epsi)

(defun smaller-scale (&OPTIONAL (epsi 1.0)) (if (> (+ 1.0 epsi) 1.0) (smaller-scale (/ epsi 10)) epsi))
(defun bigger (epsi inc-unit) (if (< (+ 1.0 epsi) 1.0) (bigger (+ epsi inc-unit) inc-unit) epsi))
(defun smaller (epsi dec-unit) (if (> (+ 1.0 epsi) 1.0) (smaller (+ epsi dec-unit) dec-unit) epsi))

(defun find-epsi (&OPTIONAL (search-depth 10) (epsi (smaller-scale)) (incdec-unit epsi))
  (if (= search-depth 0) epsi (find-epsi (1- search-depth) (bigger (smaller epsi incdec-unit) incdec-unit) incdec-unit)))

(format t "epsi: ~a ~%" (find-epsi))

It seems that it should be much simpler to find epsilon than I originally thought.  What do you think about the following program?

(defpackage :find-epsi (:use cl))
(in-package :find-epsi)

(defun find-epsi (&OPTIONAL (epsi 1.0))
  (if (> (+ 1.0 epsi) 1.0)  ; if the variable epsi is still significant
    (find-epsi (/ epsi 2)) ; halve it and try again
    epsi)) ; otherwise, we have found epsilon

(format t "epsi: ~a ~%" (find-epsi))

If we assume that a float is represented in memory as a (sign, mantissa, exponent) tuple, and assume a radix of 2, then we can find the machine epsilon exactly.  That is, if we can assume the machine stores floats using base-2 representations of the mantissa and exponent, then we know that:

The machine will store a value of 1 in floating point exactly - this would be stored as 1 for the mantissa, and 0 for the exponent, i.e. 1 * 2^0.
The machine will store all powers of two that it can represent using a single bit in the mantissa, and by varying the exponent.  E.g. 1/4 could be represented as 1 * (2 ^ -2).  Any representable power of two will be stored without losing information.
1 + epsi will be the smallest value greater than 1 that can be stored in the mantissa of the floating-point number.

EDIT

The second version looks much better than the first, but I believe there's an off-by-one error in the number of times you recurse in find-epsi.  I suggest that you create a test function, to see if your result is the machine epsilon:

(defun epsi-sig-p (epsi)
  (> (+ 1.0 epsi) 1.0))

You'll probably find that (is-sig (find-epsi)) is #f...  This also suggests that you can refactor (under the DRY principle) find-epsi to use this test function in find-epsi:

(defun find-epsi (&OPTIONAL (epsi 1.0))
  (if (epsi-sig-p epsi)
    (find-epsi (/ epsi 2))
    epsi))

but we didn't change the behavior to fix the calculation, yet.  For this, I'd suggest another routine, to check whether we should try the next possible epsi:

(defun next-epsi (epsi) (/ epsi 2))
(defun is-epsi-p (epsi)
  (and (epsi-sig-p epsi) (not (epsi-sig-p (next-epsi epsi)))))
(defun find-epsi (&OPTIONAL (epsi 1.0))
  (if (is-epsi-p epsi)
    epsi
    (find-epsi (next-epsi epsi))))

is-epsi-p should return #t, now.


Create a pump object from either one of pump model number or pump product id Consider this code:

class AssemblyFactory
{
    public function makeAssembly(array $params)
    {
        if (array_key_exists('modelNumber', $params))
            $pump = $pumpFactory->fromModelNumber($params['modelNumber']);
        else if (array_key_exists('productId', $params))
            $pump = $pumpFactory->fromProductId($params['productId']);
        else
            throw new \InvalidArgumentException("FAIL");
    }
}

Is there a way to write this code better using PHP facilities? For example, I considered instead of $params to include $modelNumber and $productId directly, but I am not sure that it will be appropriate, since there will only be one legally valid parameter used at any one time.

Namely I need a way to create a pump using EITHER model number OR product id.

to call:

$factory = new AssemblyFactory();
$pump = $factory->makeAssembly(array(
    'productId' => 33
));
$pump = $factory->makeAssembly(array(
    'modelNumber' => 'AAA-50'
));

You could make your code "more dynamic" by creating a whitelist of "actionable keys" and the associated method name to be called, but that seems like unnecessary convolution.

Your current method is direct and literal and probably makes sense to those on your development team.  I only have a few minor suggestions, but overall you should keep your basic structure.

It is not clear where $pump and $pumpFactory come from in your posted method, so I cannot offer advice on those variables.  I also don't know what processes are actioned by the subsequent method calls, nor do I know the value type (boolean?, array?, string?, etc) or value of the return.  A DocBlock would be a wise choice.

class AssemblyFactory
{
    /**
     * Generates assembly data based on model number or product id.
     *
     * @param array $params An array of pump details expected
     *                      to contain model number or product id.
     *
     * @throws InvalidArgumentException if $params does not contain a
     *         model number or product id.
     *
     * @return string Pump assembly name
     */
    public function makeAssembly(array $params)
    {
        if (array_key_exists('modelNumber', $params)) {
            return $pumpFactory->fromModelNumber($params['modelNumber']);
        }
        if (array_key_exists('productId', $params)) {
            return $pumpFactory->fromProductId($params['productId']);
        }
        throw new \InvalidArgumentException("FAIL");
    }
}

When seemingly simple code doesn't feel clear enough (or if you really love documentation blocks), then use DocBlocks to explain your script and avoid misinterpretation or lost time in the future.

PSR-5: PHPDoc

Here's an example that is relevant to your code: https://docs.phpdoc.org/references/phpdoc/tags/throws.html

Beyond your provided code, I feel that you should determine which lookup method is more efficient within your application (If this is a database lookup, is one column a Primary Key and another one not?).  Make the more efficient process the default one.

Alternatively, if you have two separate collections of data containing redundant data, consider merging the data storage so that only one lookup is necessary.


Simple yet efficient integer-to-object dictionaries? Sometimes I need an NSDictionary-like object that maps a few integers (non-negative) to objects. At first glance, NSMutableArray is great for this, provided that the indexes aren't too high so I came up with a quick category to allow holes in an array:

@implementation NSArray (FoundationAdditions)

-(id) objectAtCheckedIndex:(NSUInteger) index {
    if(index >= self.count) {
        return nil;
    } else {
        id result =  [self objectAtIndex:index];
        return result == [NSNull null] ? nil : result;
    }
}

@end

@implementation NSMutableArray (FoundationAdditions)

-(void) setObject:(id) object atCheckedIndex:(NSUInteger) index {
    NSNull* null = [NSNull null];
    if (!object) {
        object = null;
    }

    NSUInteger count = self.count;
    if (index < count) {
        [self replaceObjectAtIndex:index withObject:object];
    } else {
        if (index > count) {
            NSUInteger delta = index - count;
            for (NSUInteger i=0; i<delta;i++) {
                [self addObject:null];
            }
        }
        [self addObject:object];
    }
}

@end

Is there a better/easier/simpler way to do this? Preferably using something that's pre-existing in the Cocoa stack. Yes I know STL has some pretty good containers in this area, but mixing-in C++ just for this is overkill.

I would write a custom class that uses an NSMutableDictionary internally, boxing the indexes into NSNumber keys.

Depending on your particular use case it’s probably going to be both faster and more memory efficient than stuffing the unused array items with NSNull. Boxing the integer indexes into NSNumber key objects sounds like a lot of work performance-wise, but since the implementation uses tagged pointers, it should be pretty fast.

For extra points you can implement the object subscripting accessors (objectAtIndexedSubscript: and setObject:atIndexedSubscript:) and enjoy simple access syntax (foo[x]).


Make password/hash verification algorithm more efficient I am trying to guess the paswords from the etc/shadow file(it has 43 user/passwords). And I have been given some hits about the passwords:

Length is between 4 and 8 characters
There can be only 2 numbers and only at the end
Capital letters only in the beginning

So I started just with a small group composed by 4 character with 2 digits in it. But it takes so much time to process:

import crypt
import string
import itertools
import datetime

dir = "shadow3"
file = open(dir, 'r').readlines() #Read all the 43 hashes

username = []
hashed = []
c = 0
cc = 0

for x in file: #Split the hash and the username
    usr, hshd, wtf, iss, this, thing, here, doing, example = x.split(':')
    username.append(usr)
    hashed.append(hshd)
#GRUPO1 4 caracteres 2 numeros
letters = string.ascii_lowercase
digits = string.digits
grupo1=[]
group1=itertools.product(letters,repeat=2)
group1b=itertools.product(digits,repeat=2)
for x in itertools.product(group1,group1b):  #Join the possible iterations
  string=''.join([''.join(k) for k in x])
  grupo1.append(string)
print(len(grupo1))
for y in grupo1:#Get the one of the iterations and try it
  prueba=y
  for x in hashed: #Verify if that iteration is the password to any of the 43 users
    rehashed = crypt.crypt(prueba, x)
    if rehashed == x: #Password is found
        print('La contraseña del usuario ' + username[c] + ' es ' + prueba)
        cc = 1
    c = c + 1
if cc == 0: #after all iterations password is not found
    print('Lo sentimos "' + prueba + '" no es la contraseña de ningun usuario')

How can I improve the efficiency of this? I have a GTX 1070 if it helps for any kind of GPU processing.(I have no idea of this)

Just this small part of 4 characters is taking me for hours, it has still not finished.

Starting with the code in my answer to your related question, we can factor out the source of the words being guessed and encapsulate it all in functions:

def get_users_to_crack(file_name):
    users = {}
    with open(file_name) as file:
        for line in file:
            username, hashed_password, *_ = line.split(':')
            users[username] = hashed_password
    return users

def crack(users, words):
    cracked_users = {}
    for password in words:
        if not users:
            print("Cracked all passwords")
            return cracked_users, {}
        for username, hashed_password in users.items():
            if crypt.crypt(password, hashed_password) == hashed_password:
                print(f'La contraseña del usuario {username} es {password}')
                cracked_users[username] = password
                del users[username]
    return cracked_users, users

Now we just need a supplier of words. This can be either from a file, similar to your other question:

def dictionary_attack(file_name):
    with open(file_name) as file:
        for line in file:
            word = line.rstrip('\n').capitalize()
            if 4 <= len(word) <= 8:
                yield word

Or you build it yourself, according to the scheme you supplied here:

from string import ascii_lowercase as a_z, ascii_uppercase as A_Z, digits
from itertools import product

def get_chars(length):
    """Produces the allowed chars according to the scheme given
       [a-zA-Z][a-z]*(0-5)[a-z0-9]*2"""
    assert length > 2
    return [a_z + A_Z] + [a_z] * (length - 3) + [a_z + digits] * 2

def brute_force(min_length, max_length, get_chars):
    for length in range(min_length, max_length + 1):
        for word in product(*get_chars(length)):
            yield "".join(word)

You can then combine them into one nice tool:

if __name__ == "__main__":
    users = get_users_to_crack("shadow3")
    n = len(users)

    users_cracked_1, users = crack(users, dictionary_attack('out68.lst'))
    print(f"Cracked {len(users_cracked_1)} out of {n} users passwords using a dictionary attack")

    users_cracked_2, users = crack(users, brute_force(4, 8, get_chars))
    print(f"Cracked {len(users_cracked_2)} out of {n} users passwords using brute force")

    if users:
        print(f"{len(users)} users passwords not cracked")


Creating HTML table with JSON data using pure JS In the below function I'm fetching data from a JSON object, then creating and populating a HTML table with the contents. Yes the code is readable but I feel that it just looks too long winded for what it is. Any suggestions to how this could be simplified or made more concise?

function makeTable(data) {

    let table = document.createElement('table');
    table.id = 'itemData';

    let thead = document.createElement('thead');
    table.appendChild(thead);

    let tbody = document.createElement('tbody');
    table.appendChild(tbody);

    let header = document.createElement('tr');
    let dateHeader = document.createElement('th');
    let item1Header = document.createElement('th');
    let item2Header = document.createElement('th');
    let item3Header = document.createElement('th');
    let item4Header = document.createElement('th');
    let item5Header = document.createElement('th');
    let item6Header = document.createElement('th');
    let item7Header = document.createElement('th');

    dateHeader.appendChild(document.createTextNode('Date'));
    item1Header.appendChild(document.createTextNode('Item 1'));
    item2Header.appendChild(document.createTextNode('Item 2'));
    item3Header.appendChild(document.createTextNode('Item 3'));
    item4Header.appendChild(document.createTextNode('Item 4'));
    item5Header.appendChild(document.createTextNode('Item 5'));
    item6Header.appendChild(document.createTextNode('Item 6'));
    item7Header.appendChild(document.createTextNode('Item 7'));

    header.appendChild(dateHeader);
    header.appendChild(item1Header);
    header.appendChild(item2Header);
    header.appendChild(item3Header);
    header.appendChild(item4Header);
    header.appendChild(item5Header);
    header.appendChild(item6Header);
    header.appendChild(item7Header);

    thead.appendChild(header);

    for (let i = 0; i < Object.keys(data).length; i++) {

        let tr = document.createElement('tr');
        let dateCell = document.createElement('td');
        let item1Cell = document.createElement('td');
        let item2Cell = document.createElement('td');
        let item3Cell = document.createElement('td');
        let item4Cell = document.createElement('td');
        let item5Cell = document.createElement('td');
        let item6Cell = document.createElement('td');
        let item7Cell = document.createElement('td');

        dateCell.appendChild(document.createTextNode(Object.keys(data)[i]));
        item1Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item1']));
        item2Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item2']));
        item3Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item3']));
        item4Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item4']));
        item5Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item5']));
        item6Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item6']));
        item7Cell.appendChild(document.createTextNode(data[Object.keys(data)[i]]['item7']));

        tr.appendChild(dateCell);
        tr.appendChild(item1Cell);
        tr.appendChild(item2Cell);
        tr.appendChild(item3Cell);
        tr.appendChild(item4Cell);
        tr.appendChild(item5Cell);
        tr.appendChild(item6Cell);
        tr.appendChild(item7Cell);
        tbody.appendChild(tr);

    }

    document.body.appendChild(table);
}

The seven columns of items are basically treated identically.  You should handle them using a loop instead of duplicating the code seven times.


Positional change program in Java I've written a program and it needs a review by skilled programmers.

The programs goal is to change 0-s to X and X-s to 0. The rule is X or O can move only to an empty place and x or o can only "jump" over one X or O. Program looks like:

------------------
|x|x|x|x| |o|o|o|o|
------------------

User input

move from? (0-8)

3

move to? (0-8)

4

Output

 -----------------
|x|x|x| |x|o|o|o|o|
------------------

package tornasz;
import java.util.Scanner;

public class Tornasz {

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        char[] table = {'x', 'x', 'x', 'x', ' ', 'o', 'o', 'o', 'o'};
        int [] move = new int[2];
        int result;
        int counter = 0;

        do
        {
            drawPlayfield(table, counter);
            move = getValidMove(sc,table,move);
            makeMove(table,move);
            result = checkWin(table);
            counter ++;
        }
        while (result == 0);
        drawPlayfield(table,counter);
        displayWinner(result);
    }

    public static void drawPlayfield(char [] table, int counter){
        System.out.println("Lépésszám: "+counter);
        System.out.println("------------------");
        System.out.println("|"+table[0]+"|"+table[1]+"|"+table[2]+
                "|"+table[3]+"|"+table[4]+"|"+table[5]+"|"+table[6]+"|"+table[7]
        +"|"+table[8]+"|");
        System.out.println("------------------");
        counter++;
    }
    public static int [] getValidMove(Scanner sc, char [] table, int [] move)
    {
        System.out.println("Melyik mezővel lépsz? (0-8)");
        int start = sc.nextInt();
        if (start == -1)
        {
            System.exit(0);
        }
        System.out.println("Melyik mezőre lépsz? (0-8)");
        int destination = sc.nextInt();
        if (destination == -1)
        {
            System.exit(0);
        }
        while (start < 0 || start > 8 || destination < 0 || destination > 8)
        {
            System.out.println("Érvénytelen lépés!");
            System.out.println("Melyik mezővel lépsz? (0-8)");
            start = sc.nextInt();
            if (start == -1)
            {
                System.exit(0);
            }
            System.out.println("Melyik mezőre lépsz? (0-8)");
            destination = sc.nextInt();
            if (destination == -1)
            {
                System.exit(0);
            }
        }
        while (table[destination] != ' ')
        {
            System.out.println("Érvénytelen lépés!");
            System.out.println("Melyik mezővel lépsz? (0-8)");
            start = sc.nextInt();
            System.out.println("Melyik mezőre lépsz? (0-8)");
            destination = sc.nextInt();
        }

        int [] movement = new int[2];
        movement[0] = start;
        movement[1] = destination;
        return movement;
    }
    public static void makeMove(char [] table, int [] move)
    {
        table[move[1]] = table[move[0]];
        table[move[0]] = ' ';
    }
    public static int checkWin(char [] table)
    {
        if (table[0] == 'o' && table[1] == 'o' && table[2] == 'o' &&
                table[3] == 'o' && table[4] == ' ' && table[5] == 'x' &&
                table[6] == 'x' && table[7] == 'x' && table[8] == 'x')
        {
        return 1;
        }
        return 0;
    }
    public static void displayWinner(int result)
    {
        if (result == 1)
        {
            System.out.println("Gratulálok, vége a játéknak!");
        }
    }

}

Now I only need to set the movement rules. I think it will be an another method with boolean.

The move variable seems unnecessary - you pass it to getValidMove() but never use it inside the function.
The language has loops for repetitive tasks. In the drawPlayfield() you may do

for(int i=0; i<9; i++)
    System.out.println("|"+table[i]);
System.out.println("|");

instead of

System.out.println("|"+table[0]+"|"+table[1]+"|"+table[2]+
        "|"+table[3]+"|"+table[4]+"|"+table[5]+"|"+table[6]+"|"+table[7]
+"|"+table[8]+"|");

This costs 10 calls instead of two, but 1) saves strings addition, 2) makes the code easy expandable (you just need to replace the 9 with another expression).
The getValidMove() function returns just one int value. Why do you allocate a table for it? declare the return type as int instead of int [] and save unnecessary work.
There is some problem with reponsibilities of functions. Function getValidMove() not only gets a valid move, as its name says, but it also removes the chosen piece from the table with assignment table[start] = " "; That is a part of performing the actual move. However I can't see where the move is completed. Where do you do table[destination] = "x"; or table[destination] = "o";? And where do you store the shape of the piece removed by table[start] = " "; to be reinserted at destination?

Critical problems are fixed now - but one important remains:

The part to input user's move is unsafe. The first loop tests for both numbers being in the required range and repeats asking for input until data are correct – or until -1 appears, which terminates the program.

Then, however, another loop starts, which tests if the chosen destination position is free – and this loop does no longer validate values for being between 1 and 8. It also does not validate data for conforming the game rules (i.e. whether the jump is no more than 2 positions long).

IMHO there should be one loop, looking more or less like this:

do
{
    do
    {
        System.out.println("Input start position (0-8)");
        start = sc.nextInt();
        if (start == -1)
        {
            System.exit(0);
        }
    } while (start < 0 || start > 8);

    do
    {
        System.out.println("Input destination position (0-8)");
        destination = sc.nextInt();
        if (destination == -1)
        {
            System.exit(0);
        }
    } while (destination < 0 || destination > 8);

    if (destination == start)
    {
        System.out.println("Destination must differ from start!");
        continue;
    }
    if (destination < start - 2 || destination > start + 2)
    {
        System.out.println("Destination must not differ from start more than by 2!");
        continue;
    }

    if (table[destination] != ' ')
    {
        System.out.println("The destination position is not empty!");
        continue;
    }
} while (false);   // the input is OK now

Of course the two inner loops are asking for replacing them with a call to some helper function...


Code duplication for just an if condition I have two methods that are near-perfect duplicates. There is of course a code smell. The only thing that changes is the condition to increment a variable.

Language: Java7

private int pathsDFS(int currentDepth, int maxDepth, Airports s, Airports e, Predicate predicate) {
    int counter = 0;
    if (currentDepth > maxDepth) return counter;
    int newDepth = currentDepth + 1;
    for (Edge x : g.getNeighboursOf(s)) {
        if ( x.getDestination() == e) {
            counter++;
        }
        counter += pathsDFS(newDepth, maxDepth, x.getDestination(), e, predicate);

    }
    return counter;
}

and

private int pathsDFSB(int currentDepth, int maxDepth, Airports s, Airports e) {
    int counter = 0;
    if (currentDepth > maxDepth) return counter;
    int newDepth = currentDepth+1;
    for (Edge x : g.getNeighboursOf(s)) {
        if (x.getDestination() == e && currentDepth == maxDepth) {
            counter++;
        }
        counter += pathsDFSB(newDepth, maxDepth, x.getDestination(), e);
    }
    return counter;
}

What pattern can I apply to remove duplication? I was trying to apply like a Predicate class (calling apply) but that doesn't solve the fact the the values to use are known at runtime inside the for method. Could be considered a strategy pattern the entire for loop?

Just add a parameter boolean incrementOnAnyDepth and in your if condition write

if ( x.getDestination() == e && (incrementOnAnyDepth || currentDepth == maxDepth) )
{
    counter++;
}`