"Alien" distinguisher I am practicing some simple Java coding problems in an attempt to learn while doing them. I would like to know if my code is redundant/is there an easier way to accomplish the same thing?

Question: Which Alien?

  "A person who witnessed the appearance of the alien has come forward
  to describe the alien's appearance."

  The program will determine which alien has arrived. The three alien
  species that it could be is:

  TroyMartian, has at least 3 antenna and at most 4 eyes
  VladSaturnian, has at most 6 antenna and at least 2 eyes
  GraemeMercurian, has at most 2 antenna and at most 3 eyes

  Sample session (with output shown in text, user input in italics)

    How many antennas?

    2

    How many eyes?

    3

    VladSaturnian

    GraemeMercurian

  If the description does not match any of the aliens, there is no
  output.

My code:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class Alien {

public static int antenna;
public static int eye;

public static void main(String args[]) {
    try (BufferedReader in = new BufferedReader(new InputStreamReader(
            System.in))) {

        System.out.println("How many antennas?");

        antenna = Integer.parseInt(in.readLine());

        System.out.println("How many eyes?");

        eye = Integer.parseInt(in.readLine());

        if(troy(antenna, eye)) {
            System.out.println("TroyMartian");
        }

        if(vlad(antenna, eye)) {
            System.out.println("VladSaturnian");
        }

        if(graeme(antenna, eye)) {
            System.out.println("GraemeMercurian");
        }

        return;

    }

    catch (IOException e) {
        System.err.println("Error");
    }

}

public static boolean troy(int antenna, int eye) {
    if ((antenna >= 3) && (eye <= 4)) {
        return true;
    } else {
        return false;
    }

}

public static boolean vlad(int antenna, int eye) {
    if ((antenna <= 6) && (eye >= 2)) {
        return true;
    } else {
        return false;
    }

}

public static boolean graeme(int antenna, int eye) {
    if ((antenna <= 2) && (eye <= 3)) {
        return true;
    } else {
        return false;
    }

}
}

You're on the right track.  Good job using the try-with-resources block for the BufferedReader.  You could use a Scanner instead for convenience.  (You could even use Scanner.nextInt(), but it would work slightly differently: the newline in the input would be optional.)

The variables antenna and eye can be local to main(), and therefore should not be static members of the class.

In Java, a common naming convention for methods that return a boolean is isSomething() or hasSomething().  In this case, though, maybeSomething() seems more appropriate.

It is rarely necessary to write return true; or return false; explicitly.  Usually, you would be better off returning a boolean expression.

import java.util.Scanner;

public class Alien {
    public static void main(String args[]) {
        try (Scanner in = new Scanner(System.in)) {
            System.out.println("How many antennas?");
            int antenna = Integer.parseInt(in.nextLine());

            System.out.println("How many eyes?");
            int eye = Integer.parseInt(in.nextLine());

            if (maybeTroy(antenna, eye)) {
                System.out.println("TroyMartian");
            }

            if (maybeVlad(antenna, eye)) {
                System.out.println("VladSaturnian");
            }

            if (maybeGraeme(antenna, eye)) {
                System.out.println("GraemeMercurian");
            }
        }
    }

    public static boolean maybeTroy(int antenna, int eye) {
        return ((antenna >= 3) && (eye <= 4));
    }

    public static boolean maybeVlad(int antenna, int eye) {
        return ((antenna <= 6) && (eye >= 2));
    }

    public static boolean maybeGraeme(int antenna, int eye) {
        return ((antenna <= 2) && (eye <= 3));
    }
}


Simple event mediator I was trying to explain the mediator pattern to a new developer, and ended up writing a simple event mediator. Thoughts?

EventMediator

class EventMediator {
    private $events = array();

    public function attach($event, Closure $callback) {
        if(!is_string($event)) throw new \InvalidArgumentException();

        $event = strtolower(trim($event));
        if(!isset($this->events[$event])) $this->events[$event] = array();

        $this->events[$event][] = $callback;

        return $this;
    }

    public function trigger($event, $data = null) {
        if(!is_string($event)) throw new \InvalidArgumentException();

        $event = strtolower(trim($event));
        if(!isset($this->events[$event])) return false;

        foreach($this->events[$event] as $callback) $callback($event, $data);

        return true;
    }

    public function getEvents() { return $this->events; }
}

EventfulTrait

trait EventfulTrait {

    private $eventMediator;

    public function attachEventMediator(EventMediator $eventMediator) {
        $this->eventMediator = $eventMediator;
        return $this;
    }

    public function detachEventMediator() {
        $this->eventMediator = null;
        return $this;
    }

    public function getEventMediator() {
        return $this->eventMediator;
    }

    public function attachEvent($name, $callback) {
        if(!is_null($this->eventMediator)) $this->eventMediator->attach($name, $callback);
        return $this;
    }

    public function triggerEvent($name, $data = null) {
        if(!is_null($this->eventMediator)) return $this->eventMediator->trigger($name, $data);
        return false;
    }
}

Example

require "EventMediator.php";
require "EventfulTrait.php";

class Log {
    public function write($message) {
        echo "{$message}\n";
    }
}

class Eventful {
    use EventfulTrait;

    private $log;

    public function __construct() { $this->log = new Log(); }

    public function attachLoadEvent() {
        $self = $this;
        $log  = $this->log;
        $this->attachEvent(
            "load", function() use($self, $log) {
                $self->runOutOfNames();
                $log->write("log message");
            }
        );
    }

    public function doSomething() {
        $this->triggerEvent("load");
        $this->triggerEvent("event1");
    }

    public function doSomethingElse() {
        $this->triggerEvent("load");
        $this->triggerEvent("event1");
        $this->triggerEvent("event2");
    }

    public function runOutOfNames() { echo "load\n"; }
}

$eventMediator = new EventMediator();
$eventMediator->attach("event1", function() { echo "event1\n"; });
$eventMediator->attach("event2", function() { echo "event2\n"; });
$eventMediator->attach("done", function() { echo "done\n"; });

$eventful = new Eventful();
$eventful->attachEventMediator($eventMediator);
$eventful->attachLoadEvent();
$eventful->doSomething();
$eventful->doSomethingElse();

$eventMediator->trigger("load");
$eventMediator->trigger("done");

This will echo:

load
log message
event1
load
log message
event1
event2
load
log message
done

I think this is a very clean looking code base, so congrats getting that down. I also think that the the way you implemented the pattern worked out very well.

I don't have much to say, it's mostly just formatting.

Throughout, you insist on keeping block sections condensed to one line. It's perfectly okay to add the brackets and break apart the code into multiple lines.
In attach(), you return the current object. I don't see why you're doing so as in the example you didn't utilize this feature. Don't write code that's not in use yet!
You might consider moving strtolower(trim($event)) to a function with the appropriate parameters. Don't repeat yourself.
Why is !isset($this->events[$event]) returning false. I would expect an exception to be throw, that way if the code needs to watch for other errors later on, false could mean many things.
attachEventMediator I think needs a new name. You're not just attaching a mediator, you're setting it. If you had support for multiple mediators, then attach would be appropriate. Similarly, detach should be remove or delete.
Perhaps in replace of echo "{$message}\n";, you could use the sprintf function. I personally think this would clear up the line.
To improve your dependency injection  in Eventful, pass the Log as a parameter.
Will the message always be "log message"? Maybe pass that as an argument.

Overall I think the code is well-written. A couple more comments and better example code could have improved it even more!


Implement State Monad transformer in Haskell from scratch When I was studying Monad Transformer, I decided to create StateT s m a from scratch with instances for Functor, Applicative and Monad.

This is what I have:

newtype StateT s m a = StateT { runStateT :: (s -> m (a, s)) }

instance Functor m => Functor (StateT s m) where
    -- fmap :: (a -> b) -> StateT s m a -> StateT s m b
    -- which is (a -> b) -> (s -> m (a, s)) -> (s -> m (b, s))
    f `fmap` (StateT x) = StateT $ \ s -> fmap run (x s)
        where run (a, s) = (f a, s)

instance Monad m => Applicative (StateT s m) where
    -- pure :: a -> StateT s m a
    pure a = StateT $ \ s -> pure (a, s)
    -- <*> :: f (a -> b) -> f a -> f b
    -- which is StateT s m (a -> b) -> StateT s m a -> State s m b
    k <*> x = StateT $ \ s -> do
        (f, s1) <- runStateT k s -- :: m ((a -> b), s)
        (a, s2) <- runStateT x s1
        return (f a, s2)

instance (Monad m) => Monad (StateT s m) where
    return a = StateT $ \ s -> return (a, s)
    -- >>= :: StateT s m a -> (a -> StateT s m b) -> StateT s m b
    (StateT x) >>= f = StateT $ \ s -> do
        (v, s') <- x s
        runStateT (f v) s'

My original intention is to implement Functor (StateT s m) with Functor m restriction, Applicative (StateT s m) with Applicative m restriction, and Monad (StateT s m) withMonad m) restriction. However I couldn't do the Applicative case and had to use Monad m restriction instead. Is there a way to do it with Applicative m?

Thank you in advance.

Your implementation is correct. The additional comments are also welcome. I would write <*> without runState, but that's personal preference.

Keep in mind that you've re-implementend Control.Monad.Trans.State.Strict.StateT. For the lazy variant Control.Monad.Trans.State.Lazy.StateT, you would have to use lazy pattern matches in several places, so I assume you wanted to implement the strict variant.

To answer your question: no, we cannot implement Applicative (StateT s m) in terms of Applicative m. A Q&A on StackOverflow contains some hints, but let's reiterate them:

Suppose you have two values f, x and want y with the following types:

f :: StateT m s (a -> b)
x :: StateT m s a
y :: StateT m s b

We first remove the newtype:

f :: s -> m (s, (a -> b))
x :: s -> m (s, a)
y :: s -> m (s, b)

If our initial state is p, we have

f p :: m (s, (a -> b))
x   :: s -> m (s, a)

We want to use the s from f p in x to feed into x and the function to use the a. Now, let's suppose that m is an Applicative. This gives us only pure, fmap and <*> to work with. Let's try to write an expression that only uses those three functionspure, fmap and <*>:

z s = fmap (\(s', f') -> fmap (\(s'', x') -> (s'', f' x')) x s') (f s)

Let's check whether that implementation is sane. It's easier to deciper if we swap fmap's arguments:

x ~~> f = fmap f x
z s = f s                                   --  1
      ~~> (\(s', f') ->                     --  2
           x s'                             --  3
           ~~> (\(s'', x') -> (s'', f' x')  --  4
         )

We run f s to get our new state and our function.
Both are in m, so we use fmap to get in there.
We run x s' to get our new state again and our value.
Both are in m, so we use fmap yet again. However, we're using fmap inside fmap, which already tells us that we're not going to end up with a simple m ....

In (4), we end up with the correct new state s'' and the correct value f' x'. However, our type is s -> m (m (s, b)). Applicative does not provide any methods to reduce the number of m's, so we're stuck. We need to use join :: Monad m => m (m x) -> m x.

If we go back, we see that the problem arises due to x's type s -> m (s, a). If it was m (s -> (s, a)), we could simply use Applicative. Petr provides a detailed answer on SO.


HTML 5 form validation and form handling Problem Statement

  Create one form having following fields

  1) Email ( with html5 email validation).

  2) Password (It should take minimum 6 charactors ).

  3) Your project URL ( with html5 url validation).

  4) Project completion date ( with html5 date validation).

  5) Current month ( with html5 month validation).

  6) Current week (with html5 week validation).

  7) Current time (with html5 time validation).

  8) Select color (with html5 color features).

  9) Age (with html5 range features and also show the selected age ).

  10) Your hobbies (with html5 data list ).

  Expected output

Solution

        <title>Form validation</title>
        <meta charset="UTF-8">

Fill The Form

        <form method="POST" action="javascript:void(0)" enctype="multipart/form-data">
                <input type="email" name="email_id" placeholder="Email" required autocomplete="off" autofocus="on" pattern="^[A-Za-z0-9_-.]+@[A-Za-z-]{3, 4}\.[A-Za-z]{2, 3}$">

                <input type="password" name="password" placeholder="Password" required autocomplete="on" pattern="^[A-Za-z0-9_.-]{6, 10}$">

                <input type="url" name="url" placeholder="your project URL" required autocomplete="on">

                <input type="date" name="projectCompletionDate" placeholder="Project completion date" required autocomplete="off">

                <input type="month" placeholder="Current month" required autocomplete="on">

                <input type="week" placeholder="Current week" required autocomplete="on">

                <input type="time" placeholder="Current time" required autocomplete="on">

                Select color:<input type="color" required autocomplete="on">

                Age:<input type="range" required autocomplete="on">

                Hobbies:<input list="hobbies" name="hobby">
                                          <datalist id="hobbies">
                                            <option value="Soccer">
                                            <option value="Reading">
                                            <option value="Golf">
                                            <option value="Swimming">
                                            <option value="Tennis">
                                          </datalist>
                <input type="submit" value="Submit Query">
        </form>

1) Can this solution be improved?

2) Can regex patterns be avoided?

Validating HTML5 Code

It seems that you are new to HTML5, so it's a good idea to validate your code. You have 14 errors and 5 warnings in your code:

autofocus="on" should be autofocus
Your email regex is invalid (because of the - which needs to be escaped).
You can't have a placeholder for date, month, week, time (it doesn't make that much sense)
You can't use required with color or range (it's automatically set to some value, so requiring it doesn't make sense)
Options must have a value or a label

Problem Statement vs Your Code

range is probably not ideal for age, but as it's in the spec, there's nothing you can do about it. But: For me, it doesn't show the age in firefox or chrome. You probably need JavaScript to do that.
Your password pattern is quite restrictive. The spec says to set a minimum of 6 characters, which makes sense. But restricting what those characters might be, or the length of the password are not good ideas.
Your email pattern is too strict. There are TLDs that are more than 3 characters. The local part of an email can contain more than alphanum + _-., and the domain can also contain a lot more than a-z. Also, {3, 4} matches {3, 4} literally, which doesn't make that much sense. Just use the html5 email validation like the spec says.

Other

autocomplete="off" is a very good idea for passwords. But for many other inputs (generally generic inputs that a user uses often on different websites/forms, such as emails), it reduces usability.
Try to use standard names to increase readability and so that autocomplete will work (what even is an email_id? email would fit better).
autocomplete="on" is the default, so you don't really need to specify it (it clutters up your code and makes it harder to read).
Your indentation is inconsistent (2 vs 4 vs 8 spaces, and datalist is indented even more).
You should not use linebreaks to change the look of the page. That's what CSS is for (I know it's easier to just place

, and I do it too sometimes, but it would be cleaner to do it via CSS).

  Can regex patterns be avoided?

Not for the password, but for the email, it can be avoided. As the problem statements says, use HTML5 email validation (it works automatically for type email, no need for a pattern; and your pattern is way too strict, disallowing a lot of valid email addresses).


Python Tkinter OOPS code optimization I have the following oops code, I am also monitoring user activity to see if it is idle for more than 5 seconds. I am very new to oops, so just want to understand if there are better way to implement it.

#!/usr/bin/python

import Tkinter
import time

class simpleapp_tk(Tkinter.Tk):
    def __init__(self,parent):
        Tkinter.Tk.__init__(self,parent)
        self.parent = parent
        self.running = None
        self.initialize()

    def initialize(self):
        self.grid()

        self.entryVariable = Tkinter.StringVar()
        self.entry = Tkinter.Entry(self,textvariable=self.entryVariable)
        self.entry.grid(column=0,row=0,sticky='EW')
        self.entry.bind("<Return>", self.OnPressEnter)
        self.entryVariable.set(u"Enter text here.")

        button = Tkinter.Button(self,text=u"Click me !",
                                command=self.OnButtonClick)
        button.grid(column=1,row=0)

        self.labelVariable = Tkinter.StringVar()
        label = Tkinter.Label(self,textvariable=self.labelVariable,
                              anchor="w",fg="white",bg="blue")
        label.grid(column=0,row=1,columnspan=2,sticky='EW')
        self.labelVariable.set(u"Hello !")

        self.grid_columnconfigure(0,weight=1)
        self.resizable(True,False)
        self.update()
        self.geometry(self.geometry())
        self.entry.focus_set()
        self.entry.selection_range(0, Tkinter.END)
        self.after(1000, self.tick)
        self.eventbind()

    def OnButtonClick(self):
        self.labelVariable.set( self.entryVariable.get()+" (You clicked the button)" )
        self.entry.focus_set()
        self.entry.selection_range(0, Tkinter.END)

    def OnPressEnter(self,event):
        self.labelVariable.set( self.entryVariable.get()+" (You pressed ENTER)" )
        self.entry.focus_set()
        self.entry.selection_range(0, Tkinter.END)

    def reset(self, *ignore):
        self.running = None

    def tick(self, *ignore):
        if not self.running:
            self.running = time.time()
        elif time.time() - self.running > 5:
            print 'I waited 5 seconds...'
            self.running = None
        self.after(1000,self.tick)

    def eventbind(self):
        self.bind('<Key>',self.reset)
        self.bind('<Button-1>',self.reset)

if __name__ == "__main__":
    app = simpleapp_tk(None)
    app.title('my application')
    app.mainloop()

Tkinter is not just OOP, it's about http://en.wikipedia.org/wiki/Event-driven_programming . The code you posted does not yet show enough to be judged, because it's too small and the application purpose is not yet clearly visible. When code will grow, you will need to decompose it into modules, but it is good to think about the strategy before hand.

Tkinter allows to process events in event-driven style, so it's hard to see why do you need "a tick" to check if a user presses something. (I am not sure, may be it is intended this way).

One more note I can make is about style. Take alook at http://www.python.org/dev/peps/pep-0008/ It's not very important, but makes the code look much nicer.

UPDATE: MVC (model-view-controller) is very natural approach for Tkinter (as is the case with other GUI frameworks). Please, take a look at toy MVC (link from an answer to https://stackoverflow.com/questions/7638139/python-tk-with-mvc-pattern)


Non-restrictive email validation Just looking to do some super-duper basic email validation and was wondering if my code is efficient.

IE8 compatibility is a requirement.

/**
 * Email validation is futile
 * Make sure the @ symbol is not the first or last character
 */
jQuery.validator.addMethod("easyEmail", function(value, element, param) {
    value = $.trim(value);
    return this.optional(element) || (value.indexOf('@') > 0 && value.charAt((value.length-1)) !== '@');
}, "Please enter a valid email address.");

I am trying not to step on any toes since the RFC is mind-bogglingly complicated.

Your code would be slightly nicer this way:

jQuery.validator.addMethod("easyEmail", function(value, element, param) {
    if (this.optional(element)) {
        return true;
    }
    var trimmed = $.trim(value);
    var index = trimmed.indexOf('@');
    return index > 0 && index < trimmed.length - 1;
}, "Please enter a valid email address.");

Slightly nicer, because:

Check element before trimming value, as you might not use value at all.
It's a good practice to not overwrite parameter variables
It's good to cache the result of .indexOf to avoid calling twice

That said, since @user45891 found that nice link for you, I recommend you use it. I understand you only want a super-simple email validation, but this is waaaay too simple, to the point of almost uselessness.


Bucket sort in Haskell As a first step in learning Haskell I am solving this problem, which involves finding the Rith-ranked numbers, given some input list and many Ri. In an imperative language I would make a zeroed array c of length 200, increment c[h] for each height h, compute cumulative sums of c and then binary search c to determine the height corresponding to each given index. Because max_height is fixed, this has runtime linear in the size of input and bounded memory(*) excluding the input.
Here's my Haskell code:
max_height = 200
count_eq e = foldl (\c f -> if e == f then c + 1 else c) 0
counts heights = map (flip count_eq heights) [0..max_height]
first_gt e l = f l 0 where f (x:xs) i = if x > e then i else f xs (i+1)
solve heights indices = let accum = scanl1 (+) (counts heights) in
  map (flip first_gt accum) (map (subtract 1) indices)

It is correct but slow. I would like to know how to (A) reason about and (B) improve the performance. Also (C) can I achieve the same asymptotic performance as the imperative code?
(*) assuming each c[i] fits in a machine int. I believe the runtime statement holds regardless.

Lists are not for random access. As far as I know, Vectors are the modern way to have fixed-length random-access listlikes. They will allow you to do the traversing updates you want in one pass.

import qualified Data.Vector as V
counts = V.toList . V.accum (+) (V.replicate 200 0) . map (,1)

first_gt doesn't need a manual implementation:

import Safe
first_gt e = findIndexJust (> e)

And just for fun, this should only traverse the counts result once. (Requires that indices is sorted, though.)

import Control.Monad.Trans.State
solve heights = scanl1 (+) . map length . splitsOnFoo . map (subtract 1) where
  splitsOnFoo = evalState $ traverse (state . span . (<)) $ scanl1 (+) $ counts heights


Map Character to Characters Most Frequently Found With it (in list of strings) For an interview, I was tasked with writing a program that consumes a list of strings, and produces a mapping between every character in the list, and the characters found most frequently with it (let's call them Companion Characters).

For example, if it was given ["aabc", "bcdddd", "cde"], it would return {a=[b, c], b=[c], c=[b, d], d=[c], e=[c, d]}. a maps to b, c because they were together in the first word, b maps to c because they were together in two words ("aabc" and "bcddd"), while a and d were only with it for one word, etc.

To solve it, I used two HashMaps – One which mapped every character to a map of companion characters, to integers (the number of times found), and another which mapped character to the list of companion characters found most frequently with it. My code (which compiles and works) is below.

import java.util.*;

public class MaxMap {
   private Map<Character, Map<Character, Integer>> charMap;
   private Map<Character, List<Character>> maxMap;

   public MaxMap() {
       charMap = new HashMap<>();
       maxMap = new HashMap<>();
   }

   public void computeMap(List<String> strs) {
       for(String str: strs) {
           //Will contain all of the starting letters encountered in the string
           Set<Character> seenOriginal = new HashSet<>();
           for(int i = 0; i < str.length(); i++) {
               char original = str.charAt(i);
               if(!charMap.containsKey(original)) {
                   charMap.put(original, new HashMap<>());
               }
               //Haven't yet calculated the mappings for this character - and dups don't matter
               if(!seenOriginal.contains(original)) {
                   seenOriginal.add(str.charAt(i));
                   Set<Character> seenMapping = new HashSet<>();
                   for(int j = 0; j < str.length(); j++) {
                       //This is the same character, or a character previously encountered.
                       if(i == j || str.charAt(i) == str.charAt(j) || seenMapping.contains(str.charAt(j))) continue;
                       char added = str.charAt(j);
                       seenMapping.add(added);
                       int num = charMap.get(original).getOrDefault(added, 0);
                       num++;
                       charMap.get(original).put(added, num);
                       setMaxMapping(original, added);
                   }
               }
           }
       }
   }

   public void setMaxMapping(char original, char added) {
       List<Character> current = maxMap.getOrDefault(original, new ArrayList<>());
       int oldCount = 0;
       if(current.size() > 0) {
           oldCount = charMap.get(original).get(current.get(0));
           //Special case, in case the first char in list was the one adjusted
           if(current.get(0) == added) {
               oldCount--;
           }
       }

       int addedCount = charMap.get(original).get(added);
       if(addedCount > oldCount) {
           current = new ArrayList<>(Arrays.asList(added));
       } else if (addedCount == oldCount) {
           current.add(added);
       }
       maxMap.put(original, current);
   }

   public Map<Character, List<Character>> getMaxMap() {
       return this.maxMap;
   }
}

The main class which constructed and called it is

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

public class Main {
    public static void main(String[] args) {
        List<String> strings = new ArrayList<>(Arrays.asList("aabc", "bcdddd", "cde"));
        MaxMap mm = new MaxMap();
        mm.computeMap(strings);
        System.out.println(mm.getMaxMap());
    }
}

I didn't get the job, because my code wasn't optimal enough. How should I improve it?

Here are some tips to keep in mind when writing code that needs to be optimal:

The single biggest thing is: reduce the time complexity! This is by far the most significant improvement you can possibly make. Avoid iterations over the data as much as possible. For instance, every time you check if the char was added to charMap, the containsKey() function iterates over data in the Map. There aren't many letters so you can have a small data structure where the position of the data (index) indicates what character it relates to, this will remove the need to iterate to search for a specific character. In other cases you may reduce the number of iterations by sorting your data and using an efficient search algorithm such as binary search.
Arrays are the fastest data structure, use them instead of more complex data structures wherever it makes sense to do so.
Minimize the amount of data to be stored/manipulated - this can too be achieved by position-coding the data in your data structure, the position where the numbers are saved tells you what character these numbers relate to, so there are no actual Character variables being created until you need to create them, and then you can write code that will create only the chars you need to use/output.

Here is my implementation of the function you were tasked to write, I also avoided some of the logic checks you did, but getting rid of a few if statements is not a significant optimization so don't worry about it, focus on the main points I explained and see how they are implemented.

(notice the difference caused by improving the time complexity: according to my benchmarking, my implementation works about as fast as yours (1.1 times faster) with an input of 3 strings in the list, but it works ~5.4 times faster than yours when given an input list of 30 strings, the speed gap grows the bigger the input gets)

public static Map<Character, List<Character>> companionChars(List<String> strings){
    Map<Character, List<Character>> result = new HashMap<>(27, 1);
    int[] lengths = {26, 26};
    int[][] companionCounts = (int[][]) Array.newInstance(int.class, lengths);

    for(String str : strings){
        int[] charsFound = new int[26];
        for(int x = 0; x < str.length(); x++)
            charsFound[str.charAt(x) - 'a'] = 1;
        for(int x = 0; x < 26; x++)
            if(charsFound[x] == 1){
                for(int y = 0; y < 26; y++)
                    companionCounts[x][y] += charsFound[y];
                companionCounts[x][x]--;
            }
    }

    for(int x = 0; x < 26; x++){
        ArrayList<Character> chars = new ArrayList<Character>();
        int max = 1;
        for(int y = 0; y < 26; y++){
            if(companionCounts[x][y] > max){
                max = companionCounts[x][y];
                chars.clear();
                chars.add((char)('a' + y));
            }
            else if(companionCounts[x][y] == max)
                chars.add((char)('a' + y));
        }
        if(!chars.isEmpty())
            result.put(new Character((char)('a' + x)), chars);
    }
    return result;
}

Main function:

public static void main(String args[]) {
    List<String> strings = Arrays.asList("aabc", "bcdddd", "cde");
    System.out.println(companionChars(strings));
}


Read first column of a space delimited file with Java 8 stream Code:

import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

/**
 * Created by IDEA on 16/11/14.
 */
public class TestStream {
    private final static Pattern pattern = Pattern.compile("^(\\S+)\\s+");
    public static String firstWord(String s) {
        Matcher matcher = pattern.matcher(s);
        if(matcher.find()) {
            return matcher.group();
        }
        return null;
    }
    public static void main(String[] args) throws Exception {
        List<String> x =  Files.lines(Paths.get("/tmp/testfile"), Charset.defaultCharset()).map(line -> firstWord(line)).collect(Collectors.toList());
        System.out.println(x);

    }
}

Your code will fail with a single-column input like:

one
two
three
four

Additionally, it will produce odd results with space-prefixed lines like:

normal line
   spaceprefixed line

The reason it will fail is because the Files.lines method trims the line terminators off of each line, so, the input to the regex will be just one, etc. Since there's no whitespace after the one, the match will fail, and the method will return null.

You should consider an alternative approach of doing a limited split, and returning the first value:

private final static Pattern pattern = Pattern.compile("\\s+");

public static String firstWord(String s) {
    return pattern.split(s, 2)[0];
}

This also changes the logic slightly because empty lines will return empty-string, instead of null.

Additionally, your code will match "     spaceprefixed" from "     spaceprefixed line" whereas my code will consider the leading space to be a separator (and not part of the first word), and will match the empty string "" from "     spaceprefixed line"

In other words, I disagree with some of the handling you have with edge cases.

I would handle the edge cases in a different way in the stream too:

List<String> x = Files.lines(Paths.get("/tmp/testfile"), Charset.defaultCharset())
            .map(line -> firstWord(line))
            .filter(word -> !word.isEmpty())
            .collect(Collectors.toList());


Ruby: Database Authentication Module I have created a module that validates the credentials against different databases.
module Authentication

  DATABASES = %w[mysql mssql oracle].freeze

  DATABASES.each do |database|
    define_method("#{database}_connect") do |args|
      client = case database
               when 'mysql' then Mysql2::Client.new args.merge(ssl_mode: :disabled)
               when 'mssql' then TinyTds::Client.new args
               when 'oracle' then OCI8.new(args[:username], args[:password], "//#{args[:host]}:#{args[:port]}/#{args[:service_name]}")
               end
      case database
      when 'mysql', 'oracle' then true
      when 'mssql' then client.active?
      end
    end
  end

  def database_authentication_success?(database:, host:, port:, service_name:, username:, password:)
    options = {host: host, port: port, service_name: service_name, username: username, password: password}
    case database
    when 'mysql' then mysql_connect(options)
    when 'mssql' then mssql_connect(options)
    when 'oracle' then oracle_connect(options)
    end
  end
end

Please let me know, how can it be improved?

What is the use case and rationale for this module? I can see what it does, but I'm struggling to understand why I'd want to do it.
The fact that you have three almost identical case statements to drive the behaviour is a 'code smell'. In most other OO languages you'd use inheritance or an interface and a factory method to determine which subclass of an object to create (with a single case statement). In that case, the objective is to abstract away the differences between connecting to the databases behind an interface (which is what you seem to be trying to do in method database_authentication_success?). But the metaprogramming code generates explicitly-named methods for connecting to each type of database, which seems to do the opposite and makes it harder to use.


Binary to ASCII converter This is a program I wrote to convert binary to ASCII. I am still learning ARM assembly. How can I improve my program? Are there any bad programming habits in my code?

I've tested it on Raspberry Pi and Android.

@
@ ARM assembly programm which converts binary to ASCII
@ written by Kyle Kersey,  (C) 2015
@

.global main

main:
    adr r2, content     @ load address of binary string
    mov r3, #0          @ bit counter
    mov r4, #0          @ byte value
    b loop

loop:
    ldrb r0, [r2], #1   @ load next byte from memory
    cmp r0, #0          @ check for null terminator
    beq end_loop        @ quit loop

    sub r0, r0, #48     @ convert '0' or '1' to 0 or 1 to a bit
    lsl r4, r4, #1      @ byte<<1
    add r4, r4, r0      @ byte+=bit

    add r3, r3, #1      @ increment bit counter

    cmp r3, #8          @ if at 8th bit
    moveq r0, r4        @ move byte to r0 for printing
    moveq r4, #0        @ clear byte
    moveq r3, #0        @ clear bit counter
    bleq printChar      @ print byte, branch with link

    b loop              @ continue loop, branch without link

end_loop:
    mov r0, #10         @ ascii new line
    bl putchar          @ print new line
    mov r7, #1          @ return 0
    swi 0               @ call software interupt 0

printChar:
    push {r2, lr}       @ store content and return address
    bl putchar          @ print ascii character stored in r0
    pop {r2,lr}         @ restore the content and return address
    mov pc, lr          @ set the return address

charFormat:
    .asciz "%c"
    .align 4

content:
    .ascii "0100100001100101011011000110110001101111001000000101011101101111011100100110110001100100\000"

XOR instead of MOV
In your main subroutine, you move the value 0 into two registers.
A more efficient way to do that is to XOR the register with itself, like so:
xor r3, r3
Unnecessary branching
In your main subroutine, you branch over to the loop subroutine.
Since the loop subroutine is right beneath the main subroutine, you do not need a branch there since execution will just "fall through" to the loop subroutine.
Labels
I'm not sure how this works in ARM, but you seem to be using only global labels in your program, when there should be some local labels.
Local labels exist inside global labels, and they act kind of like a subroutine for a subroutine.
A change I might make is make the end_loop label a local label.
For more information on local labels, see here
For information on the syntax of a local label, see here
Disclaimer: I am not too familiar with ARM assembly, although I am familiar in other assemblies; some things I tell you might not work


Calculating the Factorial of a number I am looking for feedback for the implementation for calculating factorial of a number. I have tried to do with a loop, recursion and in a mutlithreaded way.

package org.spituk.study.programs;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

/**
 * Class for calculating factorial of a number.
 *
 * @author Karan Khanna
 * @version 1.0
 * @since 12/8/2018
 */
public class Factorial {

    private final static int JOB_SIZE = 10;

    private final static int AVAILABLE_PROCESSORS = Runtime.getRuntime().availableProcessors();

    /**
     * Calculates the factorial of the number with loop.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialWithLoop(final int number) {
        long factorial = 1;
        for (int index = 2; index <= number; index++) {
            factorial = factorial * index;
        }
        return factorial;
    }

    /**
     * Calculates the factorial of the number with recursion.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialWithRecursion(final int number) {
        return factorialRecursion(number);
    }

    /**
     * Calculates the factorial of the number with multiple threads.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialMultiThreaded(final int number) throws Exception {
        ExecutorService executor = Executors.newFixedThreadPool(AVAILABLE_PROCESSORS);
        List<PartialFactorialJob> partialFactorialJobs = new ArrayList<PartialFactorialJob>();

        int startIndex = 1;
        int pivot = JOB_SIZE;

        while(pivot <= number) {
            if(pivot + JOB_SIZE > number) {
                pivot = number;
            }
            partialFactorialJobs.add(new PartialFactorialJob(startIndex, pivot));
            startIndex = pivot + 1;
            pivot = pivot + JOB_SIZE;
        }

        try {
            long factorial = 1;
            final List<Future<Long>> futureList = executor.invokeAll(partialFactorialJobs);
            for (Future<Long> future : futureList) {
                factorial = factorial * future.get();
            }
            executor.shutdown();
            return factorial;
        } catch (InterruptedException | ExecutionException e) {
            throw new Exception("Factorial couldn't be calculated.");
        }
    }

    private long factorialRecursion(final int number) {
        if (number <= 0) {
            return 1;
        }
        return number * factorialRecursion(number - 1);
    }

    private class PartialFactorialJob implements Callable<Long> {

        private final int startIndex;
        private final int endIndex;

        PartialFactorialJob(final int startIndex, final int endIndex) {
            this.startIndex = startIndex;
            this.endIndex = endIndex;
        }

        public Long call() {
            long partialFactorial = 1;
            int temp = startIndex;
            while (temp <= endIndex) {
                partialFactorial = partialFactorial * temp;
                temp++;
            }
            return partialFactorial;
        }
    }
}

A few comments:

import java.util.concurrent.*;

Never do wildcard imports unless you're doing a one-time, throw-away program. Maybe, in Java 20 or so, they introduce a class java.util.concurrent.Factorial (contrived example), and then you have a name collision with one of your classes. Better stick to individual imports (or have your IDE organize the imports).

Thumbs up for attaching javadoc to all public elements.

public long getFactorialWithRecursion(final int number) {
    return factorialRecursion(number);
}

I'd skip the indirection from getFactorialWithRecursion() calling factorialRecursion() and write

public long getFactorialWithRecursion(final int number) {
    if (number <= 0) {
        return 1;
    }
    return number * getFactorialWithRecursion(number - 1);
}

Other SO members already commented that computing a factorial with multiple threads might not be a useful application, but I understand that this is for exerecise purposes.

The while loop in getFactorialMultiThreaded makes it hard to read and understand the code, with the two loop variables startIndex and pivot having their operations spread over many lines of code. I'd write something like

for (int startIndex = 1; startIndex <= number; startIndex += JOB_SIZE) {
    int endIndex = Math.min(startIndex + JOB_SIZE - 1, number);
    partialFactorialJobs.add(new PartialFactorialJob(startIndex, endIndex));
}

Similar thing applies to the while loop in your call method, I'd prefer a for loop instead.

As your PartialFactorialJob class doesn't access anything from the enclosing Factorial instance, you should make it static (otherwise it carries around a useless internal reference to its enclosing instance, typically visible in your debugger as this$1 or similar).


Deleting node from LinkedList I have this function to remove nodes that matches a certain criteria in the list. Initially when I wrote it, head node was not deleted if head->value was >= delete_value, but other nodes in the list were deleted, so I decided to write the last if statement to only delete and amend the list if head->value was >= delete_value.
It works, but I'm not sure if this is normal?

void remove(List * & head, double value)
{
   if (head == NULL)
   {
    cout << "Empty List";
    return;
   }

   if (head != NULL && head->value != delete_value)
   {
    List* temp1 = head;
    List* temp2 = temp1->next;

       while (temp2 != NULL)
       {
          if (temp2->value >= delete_value)
          {
            List* temp3 = temp2;
            temp2 = temp2->next;
            temp1->next = temp2;
            delete temp3;
          }
          else
          {
            temp1 = temp2;
            temp2 = temp2->next;
          }
       }
   }

   //This deletes head node, if value is greater than or equal to delete_value
   if (head != NULL && head->value >= delete_value)
   {
    List*temp = head;
    head = head->next;
    delete temp;
   }
return;
}

I believe these two lines are causing all the trouble.

 List* temp1 = head;
 List* temp2 = temp1->next;

temp2 never gets the head node because it is initialized as head->next to begin with.

I prefer double pointers since I find them rather cleaner.

void remove(Node<ItemType> * & head, double value) {
    Node<ItemType> **pp = &head;
    Node<ItemType> *curr = head;
    while (curr) {
        if (curr->item == value) {
            *pp = curr->next;
            delete curr;
            curr = *pp;
        }
        else {
            pp = &curr->next;
            curr = curr->next;
        }
    }
}


Can my responsive Nav be made simpler? I've created a responsive nav but feel there's probably a more simpler way of doing it.

HTML

    toggle

Link

Link

Link

Link

Link

Link

Google

Link

Link

Link

CSS

ul {
    list-style: none;
    color: #000;
}

.main-nav ul li {
    display: inline;
    padding-left: 28px;
    float: right;
}

.menu-toggle {
    float: right;
}

.menu-side {
    background-color: #333;
    border-right: 1px solid black;
    color: white;
    position: fixed;
    top: 0;
    right: -231px;
    width: 210px;
    height: 100%;
    padding: 10px;
}

.menu {
    overflow-x: hidden;
    position: relative;
    right: 0;
}

.menu-open {
    right: 231px;
}

.menu-open .menu-side {
    right: 0;
}

.menu-side,
.menu {
    -webkit-transition: right 0.2s ease;
    -moz-transition: right 0.2s ease;
    transition: right 0.2s ease;
}

@media (min-width: 700px)  {
    .menu-side, .menu-toggle {
        display: none;
    }
}

    @media (max-width: 700px)  {

    .main-nav {
        display: none;
    }
}

There is some jQuery to action this but I'm not concerned with this at the moment. I'm aware that SASS may simplify this but I'm not comfortable with that yet.

Multiple nav elements

While multiple nav elements are allowed for any given section, it looks dirty to me.  Consider using a single nav instead.  If you need an extra element as a style hook, then use div or section.

Duplicate content

Having content that is only visible to mobile users and similar content that's only visible for desktop users is code smell.  This one is extra bad because it uses identical markup for both.  Just use one set of markup and have CSS style it differently depending on the viewport.

Using px for the width of text elements

Text is not something you as a designer have any control over, all you can do is make suggestions.  An element that's 210px wide might look good for text that's 16px Times, but what happens when the user has specified that they need 20px Segoe because their eyesight isn't as good as it used to be?  Using em/rem is a better choice in most situations.

Using px for media queries

When you use px for your media queries, they'll fall apart when you start zooming (not sure if this is still the case).  Using ems are more reliable, especially if you're following my last point.

See also: http://bradfrost.com/blog/post/7-habits-of-highly-effective-media-queries/

Redundancy

When you float an element, most display values are ignored, including inline.

.main-nav ul li {
    display: inline;
    padding-left: 28px;
    float: right;
}

On a relatively positioned element, a top/right/bottom/left value of 0 does nothing.  I don't see anywhere where this is overriding a previous setting either.

.menu {
    overflow-x: hidden;
    position: relative;
    right: 0;
}


Create an object based on data from a text file The following method reads specially formatted text (TOML) from a file and parses the input using a parser to get a few properties, which are then used to create an object.

The profiler tells me that this method is the bottleneck in my program. While parsing ~5000 files, this method eats up over 95% of the total running time. While in another method, I am able to write the data parsed from this method to a text file in under 4 seconds for more than 5000 files, this method takes around 15 seconds on an average to run.

/**
* Creates an appropriate instance of a Parsable implementation depending
* upon the header of the file.
*
* @param file the path of the file from which to create a Parsable.
* @return the created Parsable.
*/
private Parsable createParsable(Path file) {
    Toml toml = new Toml();
    try (BufferedReader br = Files.newBufferedReader(file, StandardCharsets.UTF_8)) {
        StringBuilder header = new StringBuilder();
        String line;
        while ((line = br.readLine()) != null && !line.equals(HEADER_DELIMITER)) {
        header.append(line).append("\n");
        }
        toml.parse(header.toString());
        String title = toml.getString("title");
        author = toml.getString("author") != null ? toml.getString("author") : author;
        String date = toml.getString("date");
        String slug = toml.getString("slug");
        LocalDate publishDate = LocalDate.parse(date, DateTimeFormatter.ofPattern(config.getDateFormat()));
        String layout = toml.getString("layout");
        List<String> tag = toml.getList("tags");
        StringBuilder content = new StringBuilder();
        while ((line = br.readLine()) != null) {
            content.append(line).append("\n");
        }
        if (layout.equals("post")) {
            return new Post(title, author, publishDate, file, content.toString(), slug, layout, tag);
        }
        else {
            return new Page(title, author, file, content.toString(), slug, layout, tag);
        }
    }
    catch (IOException ex) {
            Logger.getLogger(DirectoryCrawler.class.getName()).log(Level.SEVERE, null, ex);
    }
    return null;
}

It is important to note that this method gets called over 5000 times in my test.
I have tried analyzing separate parts of the method for the performance problem, but haven't been able to identify any. How can I write this better?

The TOML library is from here : https://github.com/mwanji/toml4j

And the implementation of the constructors are like so:

/**
 * Creates a Post with the given paramenter.
 *
 * @param titl the post title
 * @param auth the post author
 * @param dat the post date
 * @param loc the post's Path
 * @param cont the post's content
 * @param slu the post slug
 * @param lay the layout
 * @param tag the list of tags
 */
public Post(String titl, String auth, LocalDate dat,
            Path loc, String cont, String slu, String lay, List<String> tag) {
    title = titl;
    author = auth;
    //TODO add summary option
    //this.summary = summ;
    date = dat;
    location = loc;
    content = cont;
    slug = slu;
    layout = lay;
    tags = tag;
}

I see that the processing you're doing here does not depend on the order you're processing in. This means you can parallelize the processing heavily.

Additionally you're doing line-by-line processing, which allows you to use one of the new features of Java 8, namely Files.lines

This greatly simplifies the code you have to following outline:

try (Stream<String> lines = Files.lines(path).parallel()) {
   // do line by line processing
} catch (IOException e) {
   // sensible handling
}

Also it might be faster to keep parsables in "one" file, this reduces channel and OS waiting overhead for Open/Close operations when doing I/O


Calling a method to draw parallel lines This question is a followup to the last one.

I followed RobH's advice and his edge case bug found in my code with this solution:

private Line CreateParallelLine(Line target, int offset)
{
    var parallelLine = new Line();
    var xDifference = target.point1.x - target.point2.x;
    var yDifference = target.point1.y - target.point2.y;
    var length = Math.Sqrt(Math.Pow(xDifference, 2) + Math.Pow(yDifference, 2));

    parallelLine.point1.x = (float)(target.point1.x - offset * yDifference/length);
    parallelLine.point2.x = (float)(target.point2.x - offset * yDifference/length);
    parallelLine.point1.y = (float)(target.point1.y + offset * xDifference/length);
    parallelLine.point2.y = (float)(target.point2.y + offset * xDifference/length);

    return parallelLine;
}

Now, since his solution only provided a means of creating one parallel line, even though I need to draw several lines, I have this method that draws the parallel lines (it works) but it is long and there is repeat code:

private void DrawParallelLines(object sender, LineDrawingEventArgs e)
    {
        Line referenceLine = CameraObject.GetComponent<DrawLines>().GetLineParams(ViewType + StickLocationEnum.ToString() + linename);
        Line ParallelLine1 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, 50);
        Line ParallelLine2 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, -50);
        Line ParallelLine3 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, 100);
        Line ParallelLine4 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, -100);

        LineParams paramForParallelLine = new LineParams(ViewType + "ParallelLine", Color.green, Color.green, EndPointType.Circle, EndPointType.Circle, lineThickness, endPointThickness);
        Line parallellineWithParams1 = new Line(ParallelLine1.point1, ParallelLine1.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams2 = new Line(ParallelLine2.point1, ParallelLine2.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams3 = new Line(ParallelLine3.point1, ParallelLine3.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams4 = new Line(ParallelLine4.point1, ParallelLine4.point2, paramForParallelLine, ElementType.Line);

        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams1);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams2);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams3);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams4);
        CameraObject.GetComponent<DrawLines>().UpdateAllLabels();
    }

What is the best way to refactor this method to remove redundant code?

How would you handle 300 parallel lines?

    Line ParallelLine1 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 50);
    Line ParallelLine2 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -50);
    Line ParallelLine3 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 100);
    Line ParallelLine4 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -100);
    ...
    Line ParallelLine297 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 950);
    Line ParallelLine298 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -950);
    Line ParallelLine299 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 1000);
    Line ParallelLine300 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -1000);

Something's not right isn't it?

I think you need to put words on the concepts involved here:

You have a bunch of lines.
Lines are drawn in pairs.
Lines are offset from the referenceLine by a fixed interval - one line with a positive offset, the other with a negative offset.

You need to write a method that can create n lines and return an IEnumerable<Line> given:

A referenceLine.
An offsetIncrement for the offset.
The n number of pairs of lines you want to create.
A LineParams instance.

And then you can just iterate the result and write the AddNewLine() call only once.


Dynamically-expanding array in C I'm learning C, specifically OOP in C. As practice, I wrote a dynamically-expanding array.

dynarray.h

#ifndef DYNARRAY_H
#define DYNARRAY_H

typedef struct DynArray {
    int length,
        capacity,
        *arr;

    void (*extend)(struct DynArray *dynArr);
    void (*insert)(struct DynArray *dynArr, int val);
    void (*print) (struct DynArray *dynArr);
} DynArray;

void DynArray_extend(DynArray* dynArr);
void DynArray_insert(DynArray* dynArr, int value);
void DynArray_print(DynArray* dynArr);

#endif

dynarray.c

#include <stdlib.h>
#include "dynarray.h"

DynArray* createDynArray() {
    DynArray* newDynArray = malloc(sizeof(DynArray));
    newDynArray->capacity = 10;
    newDynArray->arr = malloc(sizeof(int)*newDynArray->capacity);
    newDynArray->length = 0;

    newDynArray->extend = DynArray_extend;
    newDynArray->insert = DynArray_insert;
    newDynArray->print  = DynArray_print;
}

void DynArray_extend(DynArray* dynArr) {
    int newCapacity = dynArr->capacity * 2,
        *newArr = malloc(sizeof(int)*newCapacity),
        i;

    dynArr->capacity = newCapacity;

    for (i = 0; i < dynArr->length; i++) {
        newArr[i] = dynArr->arr[i];
    }
}

void DynArray_insert(DynArray* dynArr, int val) {
    if (dynArr->length == dynArr->capacity) {
        dynArr->extend(dynArr);
    }

    dynArr->arr[dynArr->length] = val;
    dynArr->length = dynArr->length + 1;
}

void DynArray_print(DynArray* dynArr) {
    int i;
    for (i = 0; i < dynArr->length; i++) {
        printf("%d\n", dynArr->arr[i]);
    }
}

And this is an example application of the dynamic array (program.c):

#include <stdio.h>
#include "dynarray.c"

int main( void ) {
    int i = 0;
    DynArray* someDynArray = createDynArray();

    printf("Length  : %d\n", someDynArray->length);
    printf("Capacity: %d\n", someDynArray->capacity);

    for (i = 0; i < 80; i++) {
        someDynArray->insert(someDynArray, i);
    }

    someDynArray->print(someDynArray);
}

I would like to know if there are any flaws in the style of implementing the abstract data type of the dynamically expanding array.

My 2 cents:

I believe createDynArray should return something.
The returned value of createDynArray might have a special value of NULL in case that one of the mallocs calls failed (this scenario should be covered by the code).
In the implementation of extend - consider using realloc instead of malloc and manual copy. If you insist on malloc perhaps using memcpy might be more elegant than a manual copy.
I'd move magic numbers to #defines: Initial Capacity (hard-coded to 10) and Growth Factor (2). Alternatively, they could be parameters to the createDynArray function
How would you free this struct? Perhaps you should add a "destructor" to be called before calling free with the struct pointer as a parameter.
In terms of functionality, other than printing this "array", how would the consuming programmer could get (or set) a specific item in this "array"?


Create dictionary with default immutable keys I've created a dictionary subclass with a set of default immutable keys.

from collections import MutableMapping

class Bones(MutableMapping):
    """Basic dict subclass for bones, that forbids bones from being deleted
    and new bones from being added."""

    def __init__(self, init_val=0):
        self.bones = {"between_hips": init_val,
                      "bicep_right": init_val,
                      "bicep_left": init_val,
                      "arm_right": init_val,
                      "arm_left": init_val,
                      "thigh_right": init_val,
                      "thigh_left": init_val,
                      "leg_right": init_val,
                      "leg_left": init_val,
                      "spine_lower": init_val,
                      "spine_upper": init_val}

    def __getitem__(self, item):
        return self.bones[item]

    def __setitem__(self, key, value):
        if key in self.bones:
            self.bones[key] = value
        else:
            raise KeyError("Can't add a new bone!")

    def __delitem__(self, key):
        raise TypeError("Can't delete bones")

    def __iter__(self):
        return iter(self.bones)

    def __len__(self):
        return len(self.bones)

Here are a few tests to check that it's working correctly:

from bones import Bones

bn = Bones()
bn["between_hips"] = 1

# these should all raise errors
bn["foo"] = 1
del bn["between_hips"]

Is this the proper way to create the subclass?

Background

My search to do this correctly started at the StackOverflow question "How to 'perfectly' override a dict" where I discovered there were two approaches to doing this. One was sub-classing an Abstract Base Class from the collections module and the other was actually sub-classing dict.

The justification for actually sub-classing dict confused me. Specifically the discussion of the object properties of __dict__ and __slots__. As best as I can tell from this question, I should be using __slots__, but I'm super unclear how.

When reading the title of the question, I wondered: "Why use a dict subclass when a regular class with __slots__ can do?". And judging by the interface, it seems that it may fit.

Slots defines attributes names that are reserved for the use as attributes for the instances of the class. However, no more attributes can be added to the instances when defining __slots__ on the class:

>>> class Example:
...     __slots__ = ('a', 'b', 'c')
...
>>> e = Example()
>>> e.a = 3
>>> e.d = 4
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Example' object has no attribute 'd'
>>> e.a
3

However, __slots__ are not auto-populated and doing so at class level is prohibited:

>>> class Example:
...   __slots__ = ('a', 'b', 'c')
...   a = 3
...   b = 4
...   c = 5
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: 'a' in __slots__ conflicts with class variable

You can, however, still define them in __init__:

class Bones:
    __slots__ = (
        'between_hips',
        'bicep_right',
        'bicep_left',
        'arm_right',
        'arm_left',
        'thigh_right',
        'thigh_left',
        'leg_right',
        'leg_left',
        'spine_lower',
        'spine_upper',
    )

    def __init__(self, default_value=0):
        for attribute in self.__slots__:
            setattr(self, attribute, default_value)

Now the only thing left is to add a dictionary-like interface (and/or inherit from collections.abc.Mapping):

    def __getitem__(self, key):
        return getattr(self, key)

    def __setitem__(self, key, value):
        setattr(self, key, value)

    def __len__(self):
        return len(self.__slots__)

    def __iter__(self):
        return iter(self.__slots__)

    def items(self):
        for attribute in self.__slots__:
            yield attribute, getattr(self, attribute)

Or not necessarily, since you can still access the __slots__ as regular attributes, it all depend on your use case.

Note that with this implementation, you can still delete values, which you try to prohibit in your original implementation:

>>> b = Bones()
>>> b.bicep_right
0
>>> b['arm_left'] = 6
>>> b.arm_left
6
>>> del b['leg_left']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: __delitem__
>>> del b.leg_left
>>> b.leg_left
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: leg_left

If you wish to prevent that, you need to define __delattr__ on your class as well:

    def __delattr__(self, key):
        raise AttributeError(key)


Matching two lists of dicts, strictly and more loosely Two lists of dictionaries. Dicts in A and B can be matched by a particular dict key that may be identical or similar. The result should be a list of the source key from A, the matching key from B, and an additional key from B.

Both lists are constructed using screen-scraping, so you normally don't get to see them (unless you print them explicitly). For clarity though: list A is called stoxlist and looks something like this:

stoxlist = [{'stock': 'Apple',
             'last_price': afloat,
             'some-more': 'value',
             ..
             },
            {'stock': 'Google',
             'last_price': afloat,
             'some-more': 'value',
             ..
             },
             ...
            ]

List B looks like this:

symbollist = [{'name': 'Apple',  'symbol': 'APPLE'},
              {'name': 'Google', 'symbol': 'ALPHABET'},
              ...
             ]

The following does what it should do. Using Python 2.7.

# lookup helpers
def match(this, that):
    ''' returns True if this loosely matches with that '''
    if this.strip('.') in that or this.split()[0] in that:
        return True
    else:
        return False        # this is not necessary, but explicit is better than implicit.

def fetch(name, symbols, key):
    ''' find name by dict[key] in list of dicts symbols  '''
    return next((item for item in symbols if match(name, item[key])), None)

def adddict(entry, symbol):
    dict = {'source'    : entry['stock'],
            'found'     : symbol['name'],
            'symbol'    : symbol['symbol']}
    return dict

def lookup(stox, symbols):
    '''lookup symbol for stock name'''
    hits = []
    misses = []
    for entry in stox:
        # try an exact match first
        sym = next((item for item in symbols if entry['stock'] == item['name']), None)
        try:
            # save a hit, if any
            hits.append(adddict(entry, sym))
        except TypeError:
            # if not, try a looser match
            sym = fetch(entry['stock'], symbols, 'name')
            try:
                hits.append(adddict(entry, sym))
            except TypeError:
                # try yet another possibility
                sym = fetch(entry['stock'].upper(), symbols, 'symbol')
                try:
                    hits.append(adddict(entry, sym))
                except TypeError:
                    # lastly try one of the hardcoded SPECIALS.
                    sym = fetch(entry['stock'], SPECIALS, 'name')
                    try:
                        hits.append(adddict(entry, sym))
                    except TypeError:
                        misses.append(entry['stock'])

    return hits, misses

def main():
    # load_all loads a pickle, made by some other module
    stoxlist = load_all()

    # get_symlist is a screen-scraper, using BeautifulSoup.
    symbollist, nosyms = get_symlist()

    hits, misses = lookup(stoxlist, symbollist)

I think try-except makes for better readability than if-elif. But it's four levels deep, which is not pretty.

There are four full iterations over list B (symbols) for every entry in list A (stox). That's not pretty either. Yet I can't think of any better way, because it is necessary to first try an == on all items in B, then a looser match on all items in B, and so on.

I'm not really bothered by performance yet. The lists aren't big, less than 100 items each. But perhaps it could all be done much more efficiently/elegantly/Pythonically if I chose a different data structure than list of dicts?

Please comment.

Exceptions can be faster and slower than using dict.get.

If you don't care about performance then you pick whichever you prefer.
However, if you do care you have to take into account three timings;
using get, without an exception, and with an exception.
Using get is slower than try without an exception, by about 37.8%.
But try with an exception is slower than get, by about 35.7%.

So, you should pick the method on whether there will be a sizeable amount of exceptions, or not.
In this case, as you expect upto four times, you should use dict.get.

Here are my timings in Python 3.6.0:

>>> from timeit import timeit
>>> timeit('''\
try:
    v = d['key']
except KeyError:
    pass
else:
    pass
''', 'd = {}')
0.25764639688429725
>>> timeit('''\
try:
    v = d['key']
except KeyError:
    pass
else:
    pass
''', 'd = {"key": "here"}')
0.034827991199023245
>>> timeit('''\
v = d.get('key')
if v is not None:
    pass
''', 'd = {}')
0.0920266698828982

Change match to return, rather than being in the if.
Move match into fetch. It makes the code easier to read.
Changing your nested trys to a function that yields the possible symbols, makes the code more DRY, and easier to read.
Move adddict into lookup.
Possibly remove the comments.

This makes swapping between try and dict.get simpler if one is faster than the other.
And also makes the code a little more dense, whilst still being readable.
Personally, I find the code being a bit more dense makes your code more readable.
But it's not much of a change from your current code.

This can change your code to:

# lookup helpers
def fetch(name, symbols, key):
    ''' find name by dict[key] in list of dicts symbols  '''
    return next((
            item
            for item in symbols
            if name.strip('.') in item[key]
            or name.split()[0] in item[key]
        ), None)

def methods(entry, symbols):
    yield next((item for item in symbols if entry['stock'] == item['name']), None)
    yield fetch(entry['stock'], symbols, 'name')
    yield fetch(entry['stock'].upper(), symbols, 'symbol')
    yield fetch(entry['stock'], SPECIALS, 'name')

def lookup(stox, symbols):
    '''lookup symbol for stock name'''
    hits = []
    misses = []
    for entry in stox:
        for symbol in methods(entry, symbols):
            try:
                hits.append({'source': entry['stock'],
                             'found' : symbol['name'],
                             'symbol': symbol['symbol']})
                break
            except TypeError:
                continue
        else:
            misses.append(entry['stock'])

However I don't think this should be done in Python.
It looks like your extracting data from your database and filtering it in Python, rather than performing both in SQL.
And from my limited knowable of SQL this would be much easier written SQL. Assuming SQL has an equivalent of in.

But if you want to keep with Python, to change your code to use equality, rather than in.
This is as it can change your code to have linear time, rather than quadratic.
If I were to do this I'd change your code to something like:

def methods(stox, symbols):
    methods = [
        {i['name']: i for i in symbols},
        {i['symbol']: i for i in symbols},
        {i['name']: i for i in SPECIALS}
    ]
    def inner(stock):
        strip = stock.strip('.')
        split = stock.split()[0]
        yield methods[0].get(stock)
        yield methods[0].get(strip)
        yield methods[0].get(split)
        yield methods[1].get(strip.upper())
        yield methods[1].get(split.upper())
        yield methods[2].get(strip)
        yield methods[2].get(split)
    return inner

def lookup(stox, symbols):
    meth = methods(stox, symbols)
    hits = []
    misses = []
    for entry in stox:
        for symbol in meth(entry['stock']):
            if symbol is not None:
                hits.append({'source': entry['stock'],
                             'found' : symbol['name'],
                             'symbol': symbol['symbol']})
                break
        else:
            misses.append(entry['stock'])


Basic Crypto for .NET Please tell me if I'm doing this right, and what else is needed to make this hardened.

It seems like it's too simple, like there's almost nothing I'm actually doing.  For example, there are 4 or 5 stacked using statements!

Here is the code to review:

class Program
{
    static void Main(string[] args)
    {
        var enc = true;
        var fileIn = string.Empty;
        var fileOut = string.Empty;

        /* parse command line to populate enc, fileIn, fileOut */

        Console.Write("key: ");
        var key = Encoding.UTF8.GetBytes(Console.ReadLine());
        Console.Write("IV: ");
        var iv = Encoding.UTF8.GetBytes(Console.ReadLine());

        using (var aes = AesManaged.Create())
        using (var md5 = MD5.Create())
        using (var readStream = File.OpenRead(fileIn))
        using (var writeStream = File.OpenWrite(fileOut))
        using (var cryptoStream = new CryptoStream(
            enc ? writeStream : readStream,
            enc
                ? aes.CreateEncryptor(md5.ComputeHash(key), md5.ComputeHash(iv))
                : aes.CreateDecryptor(md5.ComputeHash(key), md5.ComputeHash(iv)),
            enc ? CryptoStreamMode.Write : CryptoStreamMode.Read))
        {
            if (enc) { readStream.CopyToAsync(cryptoStream).Wait(); }
            else { cryptoStream.CopyToAsync(writeStream).Wait(); }
        }
    }
}

I am not an expert in cryptography by any stretch of the imagination but I am pretty sure an IV should be randomly generated.

  " In cryptography, an initialization vector (IV) or starting variable (SV)[1] is a fixed-size input to a cryptographic primitive that is typically required to be random or pseudorandom. Randomization is crucial for encryption schemes to achieve semantic security [...] " -

  https://en.wikipedia.org/wiki/Initialization_vector


VBA faster calculating I'm newbie in VBA world. I have recently work with a spreadsheet with over 25000 rows and about 50 columns. Every time I calculate this sheet, it takes about 30 minutes to 1 hour to perform the calculation. Basically I have a row with basic formulas and the following rows just differs data from this row (i.e. the same formulas).

For example, I have a data in sheet Cal from A to N, and range from M to AR is calculated based on those data. Row 1 is "master" formulas, and traditionally I fill down this row to the end and press Save to calculate. It takes me 30mins - 1h. Then I build a macro but the problem about time has not been solved yet. The Macro is also attached in Module 1 of the file. Also the main code is:

Here is my code:

Dim n As Long, i As Long
ThisWorkbook.Activate
Sheet9.Cells(3,1).Select
n = Range(Selection, Selection.End(xlDown)).Rows.Count
With ThisWorkbook.Worksheets(9)
Range(Cells(3, 13), Cells(3, 44)).Select
Selection.Calculate
Selection.Copy
For i = 4 To n + 2
With Range(Cells(i, 13), Cells(i, 44))
.PasteSpecial Paste:=xlPasteFormulas, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
.Calculate
.Value = .Value2
End With
Next i
End With

I hope someone could review and adjust my code or suggest any ideas to perform the calculation faster. I have heard that if code is optimized properly, it just takes about 10 mins to calculation 350.000-row data.

Please help me. Any help would be appreciated.

Thank you.

Just a couple of things:

With ThisWorkbook.Worksheets(9)
Range(Cells(3, 13), Cells(3, 44)).Select

You miss a . here, should be .Range, without that your With doesn't have any effect.

  Selection.Calculate

Don't need to select before calculate, you could immediately do .Range(..).Calculate
(Is it really necessary in your logic to calculate before copy)

With Range(Cells(i, 13), Cells(i, 44))

Again, . is missing, should be .Range...

For i = 4 To n + 2
With Range(Cells(i, 13), Cells(i, 44))
.PasteSpecial Paste:=xlPasteFormulas, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
.Calculate
.Value = .Value2
End With
Next i

Do you really need to do this in sequence?? You could just do:

.Range(Cells(3,13),Cells(3,44).Copy
.Range(Cells(4,13),Cells(n+2,44).PasteSpecial Paste:=xlPasteFormulas, Operation:=xlNone, SkipBlanks:=False
.Calculate
.Range(Cells(4,13),Cells(n+2,44).Value = .Range(Cells(4,13),Cells(n+2,44).Value2

You can also find some further tips here: https://stackoverflow.com/documentation/excel-vba/1107/vba-best-practices/5925/switch-off-properties-during-macro-execution#t=201706191115061969896


jQuery module to edit HTML elements I need to turn a lot of html elements as editable elements. You will double click on the element to edit, press enter to validate (send a PUT to the RESTapi), press esc to undo.

I've written this piece of code. Are there any performance improvements to be made?

var editable = (function() {

    var DELAY = 300,
        clicks = 0,
        timer = null;

    var addEditDelegateEvents = function(element, selector, url, idInUrl, idAttr, ajaxAttr) {
        // disable the double click, and handle it in the single click envent
        element.on("dblclick", selector, function(event) {
            event.preventDefault();
        });
        element.on("click", selector, function(event) {
            event.preventDefault();
            var href = $(this).attr('href');
            clicks++; //count clicks

            if (clicks === 1) { // single click action
                timer = setTimeout(function() {
                    // For some browsers, `attr` is undefined; for others,
                    // `attr` is false.  Check for both.
                    if (typeof href !== typeof undefined && href !== false) {
                        window.location = href // follow hlink
                    };
                    clicks = 0; //after action performed, reset counter
                }, DELAY);

            } else { //double click action
                clearTimeout(timer); //prevent single-click action
                // double click action
                //transform the double clicked element into an <input>
                var originElement = $(this);
                var editElement = $(document.createElement('input'));
                editElement.val(originElement.text());
                originElement.replaceWith(editElement);
                editElement.focus();
                id = originElement.attr(idAttr);
                url = url.replace(idInUrl, id); //replace idInUrl in url by id

                editElement.on("keypress", function(event) {

                    if (event.which == 13) {
                        //send ajax call then re-populate the list
                        var newVal = editElement.val();
                        var obj = {};
                        obj[ajaxAttr] = newVal;
                        $.ajax({
                            method: "PUT",
                            url: url,
                            data: JSON.stringify(obj),
                            contentType: "application/json; charset=utf-8",
                            success: function(data) {
                                console.log('sheet updated');
                                // Very careful here : the RESTapi should return a JSON
                                // with the updated infos
                                originElement.text(data[ajaxAttr]);
                                editElement.replaceWith(originElement);

                            },
                            error: function() {
                                console.log('error on sheet update');
                                editElement.replaceWith(originElement);
                            }
                        });
                    }
                    if (event.which == 27) {
                        editElement.replaceWith(originElement);
                    }
                });
                clicks = 0; //after action performed, reset counter
            }

        });

    };
    // public API
    return {
        addEditDelegateEvents: addEditDelegateEvents
    }
})();

And I will register my events as:

editable.addEditDelegateEvents($("p.editable_sheet_date_end"), "span", "/api/v1/sheets/ID/update", "ID", "sheet_id", "end");

I'm aware of the weakness of my implementation of url,idInUrl,idAttr, ajaxAttr.

But my question is rather: is this a solid design? My primary goal is the speed (loading and interactions).

I will have around 1000 delegates to deal with.

For your module definition, I suggest the following:

;(function(ns){

  function internalFunction(...){...}

  function anotherInternalFunction(...){...}

  ns.addDelegateEvents = function(...){...};

  ns.anotherFunction = function(...){...};

  ns.yetAnotherFunction = function(...){...};

}(this.editable = this.editable || {}));

This is more from the readability standpoint and not at all different from your approach. Your closure gets a reference to an existing global or creates one. Then you simply attach to it. You don't have to define this huge object at the end of the module.

            editElement.on("keypress", function(event) {

                if (event.which == 13) {
                    //send ajax call then re-populate the list
                    var newVal = editElement.val();
                    var obj = {};
                    obj[ajaxAttr] = newVal;
                    $.ajax({
                        method: "PUT",
                        url: url,
                        data: JSON.stringify(obj),
                        contentType: "application/json; charset=utf-8",
                        success: function(data) {
                            console.log('sheet updated');
                            // Very careful here : the RESTapi should return a JSON
                            // with the updated infos
                            originElement.text(data[ajaxAttr]);
                            editElement.replaceWith(originElement);

                        },
                        error: function() {
                            console.log('error on sheet update');
                            editElement.replaceWith(originElement);
                        }
                    });
                }
                if (event.which == 27) {
                    editElement.replaceWith(originElement);
                }
            });

Separation of concerns. Don't mix DOM code with AJAX code.

Move the AJAX into another function. Make it return a promise, which the caller can listen to. The DOM operation remains in the handler. It should be something like:

// This block of code is all DOM-related, with the exception of that AJAX
// function call. The AJAX code is moved out.
editElement.on("keypress", event => {

  if(event.which === 13){
    return theAjaxFunction(...).then(data => {
      // DOM operation on success
    }, error => {
      // DOM operation on error
    });
  }
});

// Meanwhile, somewhere else in your module
function theAjaxFunction(...){
  return $ajax(...);
}

if (event.which == 13) {
    ...
}
if (event.which == 27) {
    ...
}

To avoid foot-guns, put an else to avoid JS having to evaluate the second condition. One issue that might arise here is when some rogue code accidentally changes the value of event.which somewhere before the second if statement is evaluated. You will have a case where both blocks will be executed.

var addEditDelegateEvents = function(element, selector, url, idInUrl, idAttr, ajaxAttr) {

While there's nothing wrong with a sequence of arguments, it's often very hard to remember which one is which and the order they come in. Also, it's hard to set defaults. Suggesting you accept an object for configuration.

// Your module accepting options and merging it to defaults.
var defaults = {};

var addEditDelegateEvents = function(options){
  var mergedOptions = Object.assign({}, defaults, options);

}

// Usage
editable.addEditDelegateEvents({
  element: $("p.editable_sheet_date_end"),
  selector: "span",
  url: "/api/v1/sheets/ID/update",
  idInUrl: "ID",
  idAttr: "sheet_id",
  ajaxAttr: "end"
});

element.on("click", selector, function(event) {
  ...
  editElement.on("keypress", function(event) {

Nested event handlers can be hard to debug. It requires you to perform one action before you can test the other. What I suggest is to use event delegation for this. This way, you avoid nesting handlers, allowing you to attach handlers in advance, even when the element isn't present yet.

element.on('click', selector, function(event){
  // Create edit element with class "aClassOnEditElement"
});

// Listen for our element from an ancestor element
ancestorOfEditElement.on('keypress', '.aClassOnEditElement', function(){
  // Handle edit element event.
});

var DELAY = 300,
    clicks = 0,
    timer = null;

Major nitpick, but I suggest you do a var per variable. One issue you will have with this approach is during refactoring. You cannot easily re-arrange variables easily without having to re-add var, adjust commas and semi-colons.

Try putting timer as the first variable in the group with just cut-and-paste, you'll get what I mean.


The Blacklist: Blocking Malicious domains using Bash I've made this script to automate blocking some deviant hosts on my router, and was curious if there's anything much else that can be done to make it quicker and more efficient.
Presently I'm restricted to using packages available on the Entware repository and the latest Busybox/ash shell environment. So, for example, I can't use commands like sort -S 25% --parallel=4 -u adblock.sources (though awk/gawk is faster and more memory efficient anyway).
update_the_blacklist.sh
curl -GOs https://raw.githubusercontent.com/T145/packages/master/net/adblock/files/adblock.sources

for key in $(jq -r 'keys[]' adblock.sources); do
        case $key in
        gaming | osid_basic )
                # Ignore these lists
                ;;
        * )
                url=$(jq -r ".$key.url" adblock.sources)
                rule=$(jq -r ".$key.rule" adblock.sources)

                case $url in
                *.tar.gz )
                        curl -s $url | \
                        tar -xOzf - | \
                        gawk "$rule" | \
                        sed "s/\r//g" | \
                        sed 's/^/0.0.0.0 /' >> the_blacklist.temp.txt
                        ;;
                * )
                        curl -s $url | \
                        gawk "$rule" | \
                        sed "s/\r//g" | \
                        sed 's/^/0.0.0.0 /' >> the_blacklist.temp.txt
                esac

                unset url
                unset rule
        esac
done

# Filter duplicate hosts
gawk '!a[$0]++' the_blacklist.temp.txt > the_blacklist.txt

rm the_blacklist.temp.txt
rm adblock.sources

We're missing a shebang line.  The question is tagged bash, but that seems overkill, since there's nothing here that's not standard POSIX shell.
#!/bin/sh

We append to the_blacklist.temp.txt, but don't ensure that it starts off empty, or that it's even writeable.  It's best to create temporary files using mktemp:
tmpfile=mktemp || exit $?
trap "rm $tmpfile" EXIT

Also, instead of directing final output to the_blacklist.txt, it would be more flexible to write to standard output, and let the user choose where to send it (perhaps pipe into another filter).
But there seems to be no reason to use a file here - just pipe the for loop directly into awk (or into sort -u - that may be a better choice if you're memory-constrained, as it will perform an external merge-sort if it needs to).
The duplicated sed pipe could be replaced with a single sed program of two commands:
for
    …
done |
    sed -e 's/\r//g' -e 's/^/0.0.0.0 /' | gawk '!a[$0]++'

There's a security issue here:

                    gawk "$rule" | \

We are executing downloaded content in awk.  Because we can't be sure that the adblock.sources URL cannot be replaced with a more harmful version, we should run awk in its constrained "sandbox" mode, and use the -- separator so we don't interpret the script as arguments.
                    gawk --sandbox -- "$rule" | \

These changes give me
#!/bin/sh

set -eu

sources=$(mktemp)
trap 'rm "$sources"' EXIT

curl -s -o "$sources" https://raw.githubusercontent.com/T145/packages/master/net/adblock/files/adblock.sources

for key in $(jq -r 'keys[]' "$sources")
do
    case $key in
        gaming | osid_basic )
        # Ignore these lists
        ;;
        * )
            url=$(jq -r ".$key.url" "$sources")
            rule=$(jq -r ".$key.rule" "$sources")

            curl -s "$url" |
                case $url in
                    *.tar.gz) tar -xOzf - ;;
                    *) cat ;;
                esac |
                gawk --sandbox -- "$rule"
    esac
done |
    sed -e 's/\r//g' -e 's/^/0.0.0.0 /' | sort -u

Somebody more expert with jq could probably avoid the remaining temporary file by making a single pass over the retrieved data.


Number format in JavaScript What's the best way to format a number?

If decimal is less than 2 digits, add zero to make it 2 decimals
If more than 4, truncate to 4 decimals
If 3 decimals, keep it to 3 decimals

14 => 14.00
14.1 => 14.10
14.123 => 14.123
14.12347 => 14.1234

I don't want rounding to happen

if (number !== 0) {
    nParts = number.toString().split('.');
    if (nParts[1]) {
      if (nParts[1].length > 4) {
        nParts = 4;
      } else if (nParts[1].length < 3) {
        nParts = 2;
      } else {
        nParts = 3;
      }
    } else {
      nParts = 2;
    }
}
number = number.toFixed(nParts);

Help me improve on this.

since you don't want rounding, treat as string rather than number...

function strange(number){
    var n=0;
    if (number !== 0) {
        nParts = number.toString().split(/\.|,/);
        if (nParts[1]){
            n=nParts[1].length;
            n = n <=2 ? 2 : n==3 ? 3 : 4;
            nParts[1] = nParts[1] + '0'
        }else{
            n=2;
            nParts[1]='00';
        }
    }
    return nParts[0] + '.' + nParts[1].slice(0,n);
}

;strange(14);
;strange(14.1);
;strange(14.123);
;strange(14.12347);

/*
14 => 14.00
14.1 => 14.10
14.123 => 14.123
14.12347 => 14.1234
*/


Unix sort in Clojure I am implementing Unix sort in Clojure. This is my first program, and I would like some feedback regarding any non-idiomatic, harmful code and any other best practices I can include in future versions. I tried my best to follow the coding standards, but went with camelCase instead of hyphen-case for variable and function names.

Main class: sort.core

(ns sort.core
  (:gen-class)
  (:use clojure.contrib.core)
  (:use clojure.tools.cli)
  (:require [sort.lib :as lib]))

(defn -main [& args]
  (let [[options args banner] (cli args
                                   ["-h" "--help" "Show help" :default false :flag true]
                                   ["-i" "--input" "The input file" :default nil]
                                   ["-d" "--delim"  "The delimitter" :default ","]
                                   ["-f" "--field" "The column to parse(1 indexed)" :parse-fn #(Integer. %)])
        {:keys [delim field input]} options]

    (lib/help options args banner)

    (when (= (lib/fileKind input) "file")
        (lib/printOutVector delim
                            (lib/sortVector field
                                            (lib/splitFile input delim ))))

    (System/exit 0)))

Library: sort.lib

(ns sort.lib
  (:require [clojure.string :as str])
  (:import java.io.File))

(defn splitFile
  "splits a file into a 2d array"
  [s delim]
  (pmap #(str/split % (re-pattern delim))
        (str/split(slurp s) #"\n")))

(defn printOutVector
  "prints out the 2dVector formatted by the delim arg"
  [delim coll]
  (println
    (apply str
           (interpose "\n" (map #(apply str (interpose delim %)) coll)))))

(defn sortVector
  "sorts a 2d vector by the given index"
  [index coll]
  (sort-by #(nth % index) coll))

(defn help
  "custom help message for sort.core"
  [opts args banner]
  (when (:help opts)
    (println (str "input: " (:input opts)))
    (println (str "field: " (:field opts)))
    (println (str "delim: " (:delim opts)))
    (println banner)
    (System/exit 0)))

(defn fileKind [filename]
  (let [f (File. filename)]
    (cond
      (.isFile f)      "file"
      (.isDirectory f) "directory"
      (.exists f)      "other"
      :else            "(non-existent)" )))

You should use namespace-qualified keywords instead of strings in the fileKind function:

(defn fileKind [filename]
  (let [f (File. filename)]
    (cond
      (.isFile f)      ::file
      (.isDirectory f) ::directory
      (.exists f)      ::other
      :else            ::non-existent)))

... which would require the following change in the -main function:

(= (lib/fileKind input) :lib/file)


Reference service for a cache I'm using JDK 11.

This is my code:

public class ReferenceService {

    private final transient Cache<String, Reference> cache;
    private final transient EntityManager entityManager;

    public ReferenceService(
        Cache<String,Reference> cache,
        EntityManager entityManager
    ) {
        this.cache = cache;
        this.entityManager = entityManager;
    }

    public Optional<Reference> get(String id) {
        return Optional.ofNullable(this.cache.get(id))
            .or(() -> Optional.ofNullable(this.entityManager.find(Reference.class, id)));
    }
}

I would like to know to write this code again in order to it's more elegant:

return Optional.ofNullable(this.cache.get(id))
    .or(() -> Optional.ofNullable(this.entityManager.find(Reference.class, id)));

I guess I'm using too much Optional.ofNullable each time I need to get a reference from cache or from persistence layer.

ADITIONAL CODE

private <T> Cache<String, T> getOrCreateCache(String name, Class<T> type) {
    Cache<String, T> cache = cacheManager.getCache(name, String.class, type);

    if (cache == null) {
        CompleteConfiguration<String, T> config =
            new MutableConfiguration<String, T>()
                .setTypes(String.class, type);

        cache = cacheManager.createCache(name, config);
    }

    return cache;
}

Design Fix

The correct solution to this is to make your cache responsible for retrieving the entity on cache-miss.

Consider for a moment the goals of a cache:

Keep expensive to calculate / retrieve information in a manner suitable to quick access
Abstract away cache handling (invalidation, cache-misses, ...) from consumers
Give certain guarantees for the consistency of cached information

As it stands the way this code is setting up and using the cache utterly fails on the second point. Luckily javax.caching.Cache does support loading entries into the cache on misses using a CacheLoader.

For this to happen you need to create a CacheLoader that is aware of the EntityManager used to access the entities:

[..] config = new MutableConfiguration<String, T>()
  .setTypes(String.class, type)
  .setReadThrough(true) // enables silent loading
  .setCacheLoaderFactory(new FactoryBuilder.SingletonFactory(
    new CacheLoader<String, T>() {
      @Override
      public T load(String key) {
          return entityManager.find(type, key);
      }
      @Override
      public Map<String, T> loadAll(Iterable<? extends String> keys) {
          return Stream.of(keys)
            .collect(Collectors.toMap(Function.identity(), this::load));
      }
   });

You should notice at this point that you're locking yourself into caching on a single primary key (namely String). This is something that might bite you in the backside down the road, but if you only use Strings as primary keys for entities that should be alright.

n.b. that I haven't even checked whether this code compiles.