Splicing enumerables together Basically, what I mean by splice is to take an array of Enumerables, take the 1st element from the first one, then the 1st element from the second one, and so forth, until we come back to the 1st enumerable, and we then take the next element from it and each of the others.

Sort of like this:
[A A] [B B] [C C] [D D] [E E] [F F]
Splicing these together would produce [A B C D E F A B C D E F]

namespace xofz
{
    using System.Collections.Generic;

    public interface MaterializedEnumerable<out T> : IEnumerable<T>
    {
        long Count { get; }
    }
}

A MaterializedEnumerable is basically an enumerable which has been written to memory, so it can be iterated as many times as you want.  It also gets a Count property.

Here is the actual splice code.  Don't worry about what an OrderedMaterializedEnumerable is, just think of it for now as a wrapper for List<T>

namespace xofz.Transformation
{
    using System.Collections.Generic;
    using System.Linq;
    using xofz.Materialization;

    public class EnumerableSplicer
    {
        public MaterializedEnumerable<T> Splice<T>(IEnumerable<T>[] sources)
        {
            var lists = new List<T>[sources.Length];

            // first, enumerate all the items into separate lists
            for (var i = 0; i < sources.Length; ++i)
            {
                lists[i] = new List<T>(new LinkedList<T>(sources[i]));
            }

            // then, splice the lists together
            var list = new List<T>(lists.Sum(l => l.Count));
            var smallestCount = lists.Select(l => l.Count).Min();

            for (var i = 0; i < smallestCount; ++i)
            {
                list.AddRange(lists.Select(l => l[i]));
            }

            var remainingLists = new LinkedList<List<T>>();
            foreach (var l in lists)
            {
                l.RemoveRange(0, smallestCount);
                if (l.Count > 0)
                {
                    remainingLists.AddLast(l);
                }
            }

            if (remainingLists.Count == 0)
            {
                return new OrderedMaterializedEnumerable<T>(list);
            }

            IEnumerable<IEnumerable<T>> remainingEnumerables = remainingLists;
            list.AddRange(this.Splice(remainingEnumerables.ToArray()));

            return new OrderedMaterializedEnumerable<T>(list);
        }
    }
}

This method is slightly recursive, to handle enumerables of different sizes.  I hope it is easy enough to read!

More transformations I wrote lately are available at xofz.Core on Github.

My comment is that you can write your desired method with far more generality, no recursion, and far less realization into lists. For example:

using System;
using System.Linq;
using System.Collections.Generic;

public class Program
{
    public static IEnumerable<R> Zipper<A, R>(IEnumerable<IEnumerable<A>> sequences, Func<IEnumerable<A>, R> f)
    {
        var enumtors = sequences.Select(seq => seq.GetEnumerator()).ToList();
        while(true)
        {
            // Not quite kosher, as we are using Select to cause a side effect!
            var hasValues = enumtors.Select(e => e.MoveNext());
            if (hasValues.Any(v => !v))
                break;
            yield return f(enumtors.Select(e => e.Current));
        }
        foreach(var enumtor in enumtors)
          enumtor.Dispose();
    }

    public static void Main()
    {
        foreach(var s in Zipper(
            new[] {
              new[] {1, 2},
              new[] {3, 4},
              new[] {5, 6, 7} },
            x => string.Join("-", x)))
          Console.WriteLine(s);
    }
}

Produces output

1-3-5
2-4-6

Note that my version works on infinite lists provided that there are finitely many of them.  Note also that my method is "lazy"; it only makes computations when the caller "pulls" something.


Max-heap implementation with List I am trying to create max-heap for practice.

Are there any bugs?
Are there ways that I can improve the code quality?

public class MaxHeap <E extends Comparable<? super E>>{
  List<E> list;

  public MaxHeap(E[] array){
    this(Arrays.asList(array));
  }
  public MaxHeap(List<E> list){
    if(list == null)
      throw new IllegalArgumentException("list cannot be null");
    this.list = new ArrayList<E>(list);
    buildHeap();
  }

  public void buildHeap(){
    for(int i=lastParent(); i >= 0; i--)
      maxHeapify(i);
  }

  private void maxHeapify(int parent){
    if(isParent(parent)){
      //assume left child is bigger for now
      int biggerChild = 2 * parent + 1;
      int rightChild = biggerChild+1;
      if(rightChild < size() && compare(rightChild, biggerChild) > 0)
        biggerChild = rightChild;

      if(compare(biggerChild, parent) > 0){
        swap(biggerChild, parent);
        maxHeapify(biggerChild);
      }
    }
  }

  private int compare(int fir, int sec){
    return list.get(fir).compareTo(list.get(sec));
  }

  public boolean add(E item){
    if(item == null)
      return false;

    list.add(item);
    int child = last();
    int parent = getParent(child);

    while(isParent(parent)){
      if(compare(parent, child) < 0){
        swap(parent, child);
        child = parent;
        parent = getParent(parent);
      }
      else
        break;
    }
    return true;
  }

  private void swap(int left, int right){
    Collections.swap(list, left, right);
  }

  public int size(){
    return list.size();
  }

  public E remove(){
    if(isEmpty())
      return null;

    swap(0, last());
    E max = list.remove(last());
    maxHeapify(0);
    return max;
  }

  public boolean isEmpty(){
    return size() == 0;
  }

  private int last(){
    return size() - 1;
  }

  private int getParent(int child){
    if(index == 0)
      return -1;
    else
      return (child-1)/2;
  }

  private boolean isParent(int index){
    return index >= 0 && index <= lastParent();
  }

  private int lastParent(){
    return size()/2 - 1;
  }

  public String toString(){
    StringBuilder builder = new StringBuilder();
    int lineCounter = 0;
    int lineMax = 1;
    for(E item : list){
      builder.append(item);
      builder.append(" ");
      lineCounter++;
      if(lineCounter >= lineMax){
        builder.append("\n");
        lineCounter = 0;
        lineMax++;
      }
    }
    return builder.toString();
  }
}

Just a few smaller issues:

Why expose buildHeap() when every other public method, including the constructors, already leaves you with a perfectly sorted heap?
Why does the add() method contain sorting logic? This would usually be placed in an increment() method.
An increment() method is missing.
Your add() method did not copy the element. Your heap is therefore vulnerable to side effects where any of the elements you have a reference on, did get modified in the outside world. Either warn, watch, encapsulate or make immutable.
Comments? At least your constructors and public methods should be documented, denoting which side effects are to be expected.
There is no interface to get the sorted list back.
The toString() method returns the internal order of the heap. Nice for debugging, but not what you would expect. You would expect it to return elements in lexicographical order.
Please don't use it in production. Your basic MaxHeap has significantly worse runtime characteristics than the default Fibonacci Heap.
There is a getParent() helper, but you have omitted such helpers for getLeft() and getRight(), even though you are using the corresponding expressions multiple times.


General pathfinder in Python I was recently working on a project where I had the following setup:

from stargaze.sg import SGObject
from stargaze.constants import NegInf, Infinite, PATH_CLAMPED, PATH_REVERSIBLE, PATH_ACCELERATABLE
from stargaze.utils import clamp

__all__ = [
    'PathIdentifier', '_basePath', '_clampedPath', '_reversiblePath',
    '_acceleratablePath', 'Path'
    ]

class PathIdentifier(SGObject):
    """
    INTERNATL: The base identifier class for a class object.
    """
    OBJ_REPR_PREFIX = "StarGaze Path:: "

class _basePath(PathIdentifier):
    def __init__(self, **config):
        self._time = 0
        self.speed = config['speed']

    def __move__(self):
        self._time += self.speed

class _clampedPath(PathIdentifier):

    def __init__(self, **config):
        start = config['start']
        end = config['end']
        if start > end:
            errmsg = "Start point cannot be greater " \
                     "than the end point."
            raise ValueError(errmsg)
        self._time = start
        self.end = end

        self.finished = False

    def __move__(self):
        if self.finished:
            return

        if self._time >= self.end:
            self.finished = True

class _reversiblePath(PathIdentifier):

    def __init__(self, **config):
        self.direction = 1

    def __move__(self):
        if self.direction == 1 and self._time >= 1:
            self.direction = -1
            self.speed *= -1
        elif self.direction == -1 and self._time <= 0:
            self.direction = 1
            self.speed *= -1

class _acceleratablePath(PathIdentifier):

    def __init__(self, speed, acceleration, min_speed=NegInf, max_speed=Infinite):
    def __init__(self, **config):
        self.accel = config['acceleration']

        self.min_speed = config.get('min_speed', NegInf)
        self.max_speed = config.get('max_speed', Infinite)

    def __move__(self):
        if self.accel != 1:
            self.speed = clamp(self.accel * self.speed,
                self.max_speed,
                self.min_speed)

class Path:

    def __init__(self, flags, **config):
        _basePath.__init__(self, **config)
        if flags & PATH_CLAMPED:
            _clampedPath.__init__(self, **config)
        if flags & PATH_REVERSED:
            _reversiblePath.__init__(self, **config)
        if flags & PATH_ACCELERATABLE:
            _acceleratablePath.__init__(self, **config)

        self.flags = flags

    def move(self):
        flags = self.flags

        if flags & PATH_CLAMPED:
            _clampedPath.__move__(self)

        if flags & PATH_REVERSED:
            _reversiblePath.__move__(self)

        if flags & PATH_ACCELERATABLE:
            _acceleratablePath.__move__(self)

None of the code is relevant, you won't have to be able to run any of it to answer my question. What I do in this code essentially is I create classes that define their own __init__ and __move_ methods, but I don't call them from instances of their class. Instead, I call them as class methods acting on the instance of another class. Is this bad style in Python? How else might I achieve what I was trying to do here?

You are using classes as function containers. Path doesn't inherit from any of the other classes, yet you are passing your self instance to their initializer functions.

class _basePath:
    def __init__(self, **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _basePath.__init__(self, **config)

is no different from

def _base_path_init(instance, flag **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _base_path_init(self, **config)

You do the same thing in move() to access the specific move implementations.

If you take a step back, this code looks like it is trying to combine the concept of an object model with the concept of mixins or traits. Instead, if we take a step back and focus on the object model concept, the code can be simplified.

class Path(SGObject):
  def __init__(self, flags, **configs):
    SGObject.__init__(self)
    self._traits = []
    if flags & PATH_CLAMPED:
        self._traits.append(ClampedPath(self, **config))
    if flags & PATH_REVERSED:
        self._traits.append(ReversiblePath(self, **config))
    if flags & PATH_ACCELERATABLE:
        self._traits.append(AcceleratablePath(self, **config))

  def move(self):
    for trait in self._traits:
      trait.move(self)

Then each model would have the following interface.

class PathTrait(object):
  def __init__(self, instance, **configs):
     #Initialize Instance Here
  def move(instance):
     #Alter Instance Here

Notes:

The above assumes that the order of move operations needs to be enforced by Path (ie. ClampedPath's move must occur before ReversiblePath's move). If this is not the case, the initializer can be simplified to just take a list of model classes.

class Path(SGObject):
  def __init__(self, traits, **configs):
    SGObject.__init__(self)
    self._traits = [t(self, **config) for t in traits]

This allows you more flexibility in the future and removes the needs for flags entirely.

The way to code is written now, _clampedPath introduces the concept of movement no longer being allowed. If the path is clamped and it has finished, should the other traits execute at all?


Planning Poker Using Knockout I am trying to build something similar to planning poker and am very new to Knockout and was wondering if anyone could help me improve on my very crude start?

This is what I have so far:

HTML

Selected Card

CSS

 .profile { width: 50px; height: 80px; color: #FFF; background: black; border: 1px solid   #FFF; float: left; line-height:80px }
 .highlight { background: yellow !important; border:1px solid #000; color: black; }

JavaScript/Knockout

function Step(number) {
    this.active = ko.observable(false);
    this.name = ko.observable( number);
}

var model = function () {
    var items = ko.observableArray([new Step(1), new Step(2), new Step(3), new Step(5),new Step(8),new Step(13),new Step(20),new Step(40),new Step(100)]);
    var selectedItems = ko.computed(function () {
        return _.filter(items(), function (item) {
            return item.active();
        });
    })

    var clicked = function (item) {
        items().forEach(function(item){ item.active(false) });
        item.active(!this.active());
    };

    var save = function () {
        alert("sending items \n" + ko.toJSON(selectedItems()));
    }

    return {
        items: items,
        selectedItems: selectedItems,
        save: save,
        clicked: clicked
    }
}

ko.applyBindings(model);

http://jsfiddle.net/grahamwalsh/63yeD/

You are using function as definition for model and inside that function you are defined an interface to have access to those properties/methods. In complicated cases you would have inflexible usage between definition and public interface. Better solution would be to create definition for the class like this

var Model = function () {
   var self = this;
   self.selectedItems = ko.computed(function () {
        return _.filter(items(), function (item) {
            return item.active();
        });
    })
}

and then create an instance of it

var model = new Model();

because a new operator you will create new JS object and this object will be send as scope for current this content. This is commonly used patterns in order to follow OOJ see link here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Introduction_to_Object-Oriented_JavaScript


JS, TS, handle 3 consecutive ifs / ternaries I need to compare a title for 3 strings

here what I'm doing

var textMessage = ""
props.screen == 'dd' ? textMessage = "Next, select the dd " : null
props.screen == 'ff' ? textMessage = "Next, select the ff " : null
props.screen == 'ss' ? textMessage = "Next, select the ss " : null

let textMessage = 'Next, select the '
switch (props.sceen) {
  case 'dd':
    textMessage += 'dd'
    break
  case 'ff':
    textMessage += 'ff'
    break
  case 'ss':
    textMessage += 'ss'
    break
  default:
    textMessage = ''
}

or

messageDictionary = {
  dd: 'Next, select the dd ',
  ff: 'Next, select the ff ',
  ss: 'Next, select the ss '
}
textMessage = messageDict[props.screen] || ''

By using the second way, it will be easier for you to add the new screen option.


Reversing all nested sequences I'm writing a function that returns all (nested) sequences reversed:

(defn my-reverse-v1 [input]
  (let [head (first input)
        tail (rest input)]
    (if head
      (conj (my-reverse-v1 (vec tail))
            (if (sequential? head)
              (my-reverse-v1 (vec head))
              head))
      [])))

(defn my-reverse-v2 [initial input]
  (let [head (first input)
        tail (rest input)]
    (if head
      (recur (cons (if (sequential? head) (my-reverse-v2 [] head) head) initial) tail)
      (vec initial))))

(defn test-my-reverse []
  (let [input [[1 2] 3 [4 [5 6]] 7 [8 [9 [10 11] 12]]]
        expected [[[12 [11 10] 9] 8] 7 [[6 5] 4] 3 [2 1]]]
    (println "my-reverse-v1:" (= expected (my-reverse-v1 input)))
    (println "my-reverse-v2:" (= expected (my-reverse-v2 [] input)))))

my-reverse-v2 is the tail-recursive version, whereas my-reverse-v1 isn't. These two functions work as expected, but I think there will be lots more succinct or Clojure-ish implementations for the requirements.

For an extremely short version of the (not tail-) recursive algorithm, I recommend using walk. You can write the code with walk like this:

(defn my-reverse-v3 [input]
  (walk #(if (sequential? %) (my-reverse-v3 %) %) reverse input))

For a properly tail-recursive version, my cleanest results use clojure.zip:

(defn my-reverse-v4 [input]
  (loop [z-input
         (zipper sequential? seq (fn [_ c] c) input)]
    (if (end? z-input)
      (root z-input)
      (recur (next (edit z-input #(if (sequential? %) (reverse %) %)))))))

For this solution, I followed a structure which I found at Exploration through Example.

(Incidentally, your my-reverse-v2 is not tail-recursive. The function calls itself within a call to cons. One of the nice things about Clojure's recur is that it will throw a compile error when it's not used in a tail-recursive position; you'd surely see that in action here.)


Draw a graph using braille characters I hacked together a way to make a small graph using Braille unicode codepoints by abusing unicodedata:

def plot(timeline):
  ret = []
  for r1, r2 in zip(timeline[::2], timeline[1::2]):
    character_name = "BRAILLE PATTERN DOTS-"
    dots  = [x for x, y in zip("7321", range(r1))]
    dots += [x for x, y in zip("8654", range(r2))]
    character_name += "".join(sorted(dots))
    ret.append(unicodedata.lookup(character_name) if dots else " ")
  return "".join(ret)

>>> print plot([random.randrange(0,4) for _ in range(100)])
 ⣆⣆⣶⣤⢠⡀⣦⡆⢀⡄⣰⣠⣦⣆⣰⢰⢠⢰⣄⡄ ⣀⣦⡆⣄⣶⣤⣠⡆⣠⣦⣆  ⣰⣴⡄⣤⣀ ⡀⣄⡀⣦⣶⣰⣶⣄⣴

Barring low-hanging incremental improvements such as memoizing the call to unicodedata.lookup, what would be a more proper way to perform this lookup?

Why zip?

You have:

dots  = [x for x, y in zip("7321", range(r1))]
dots += [x for x, y in zip("8654", range(r2))]

You're never using the second part of that zip. So you're basically making lists for the two ranges, making lists for the two zips, making lists for the outcomes of those things, all to drop most of the information.

You can simplify that to just:

dots = '7321'[:r1] + '8654'[:r2]

character name

Try to avoid building up strings with +=. Instead just use format:

character_name = "BRAILLE PATTERN DOTS-{}".format(join(sorted(dots)))


RTL management HTML & CSS I've been working around a solution to easily manage LTR and RTL on the same interface.

The different solutions I knew at first:

Override CSS according to direction.
Use a program to transform your CSS in LTR to a new RTL file.
Start from a body { transform: rotateY(180deg) }.

1. Override CSS according to direction

This solution is painful for developers.
You have to think of overriding ever CSS that will not flip properly.

Such as:

.title {
  float: left;
  margin-left: 10px;
}

body[dir="rtl"] .title {
  float: right;
  margin-left: 0;
  margin-right: 10px;
}

Note: Adding an attribute dir="rtl" on the <body> flips the whole interface, but you'll still need to override margins, floats, etc.

2. Use a program to transform your CSS in LTR to a new RTL file

This implies using an external program you didn't control to run on all your style files and then you have to make sure to send the LTR or RTL version accordingly to the client.

To me it felt quite unsafe to rely on this. I can't be sure it works 100% fine and it can be quite complex (from my point of view, I didn't dive into it).

3. Start from a body { transform: rotateY(180deg) }

This is the option I went for.

It felt a lot more pleasant. Almost everything is already working fine (in my test cases). I only need to work on text elements.

Here is what I came up with so far:

#app[x-dir="rtl"] {
  transform: rotateY(180deg);
}

#app[x-dir="rtl"] input,
#app[x-dir="rtl"] textarea,
#app[x-dir="rtl"] img,
#app[x-dir="rtl"] video,
#app[x-dir="rtl"] iframe,
#app[x-dir="rtl"] h1,
#app[x-dir="rtl"] h2,
#app[x-dir="rtl"] h3,
#app[x-dir="rtl"] h4,
#app[x-dir="rtl"] h5,
#app[x-dir="rtl"] h6,
#app[x-dir="rtl"] p,
#app[x-dir="rtl"] label {
  display: inline-block;
  transform: rotateY(180deg);
  direction: rtl;
}

#app[x-dir="rtl"] label input {
  transform: rotateY(0);
}

#app[x-dir="rtl"] .rtl {
  display: inline-block;
  transform: rotateY(180deg);
  direction: rtl;
}

Explanation

The only thing missing after flipping the whole interface is:

I have to flip texts back, because they are wrong(flipped with the page).
Add a direction: rtl; on each tag containing content that needs to be display as RTL.

What I'm doing:

Generalizing as much as possible tags that for sure need to have RTL behavior and flipped back.
Give the developer access to a class .rtl, for edge cases (eg: I can't guess if <span> and <div> contain text or not).

Note: I use an attribute x-dir="", because dir="" flips the interface. I want full control.

Drawbacks:

The only one I can think of: You can't add .rtl to an element that already has a transform because it'd override. It is not additive.

Feel free to test around: DEMO

Questions

Can you think of any edge case I haven't thought of?

Is it a good way to go?

Any optimization?

Helpful links

https://alfy.me/2014/07/26/lets-talk-about-rtl.html

I found a case where the solution #3 doesn't work : when you're dealing with scroll bars.

In your example, if you put a container smaller than the content, the scrollbar behave strangely in RTL, scrolling right when you press the left arrow and vice versa.

The same thing happen when you press the mouse's middle click and move to any horizontal direction.


Factory Method and Log writer class I Have this class want to practice how to write a factory method pattern
and I can change the logPlace by set the app.config,
and I try to improve the performance so I use static List<string>,I want to decrease IO loading
Here is my question

This class can call factory method pattern or not?
Is my code as clear as it could be?
I think the LogBoth class can be much better, but I have no idea how to improve it.

   public enum LogLevel
    {
        Error, Debug, Info, Fatal
    }
    static class Msg
    {
        public static List<string> msgList = new List<string>();
    }
    public class LogHelper
    {
        public static ILog Create()
        {
            string logPlace = System.Web.Configuration.WebConfigurationManager.AppSettings["LogPlace"];
            switch (logPlace)
            {
                case "DB":
                    return new LogDB();
                case "File":
                    return new LogFile();
                case "Both":
                    return new LogBoth();
                default:
                    return new LogFile();
            }
            throw new NullReferenceException();
        }
    }
    public interface ILog
    {
        void WriteLog(string msg);
        void ExecWrite(LogLevel logLevel);
    }
    public class LogDB : ILog
    {
        public LogDB()
        {
            Msg.msgList.Clear();
        }
        public void WriteLog(string msg)
        {
            Msg.msgList.Add(msg);
        }
        public void ExecWrite(LogLevel logLevel)
        {
            throw new NotImplementedException();
        }
    }
    public class LogFile : ILog
    {

        private static object _lockObject = new object();

        public LogFile()
        {
            Msg.msgList.Clear();
        }

        public void WriteLog(string msg)
        {
            Msg.msgList.Add(msg);
        }
        public void ExecWrite(LogLevel logLevel)
        {
            try
            {
                Policy.Handle<Exception>().Retry(1).Execute(()=>WriteToFile(logLevel));
            }
            catch (Exception ex)
            {
                LogDB logDB = new LogDB();
                logDB.ExecWrite(logLevel);
            }
        }

        private void WriteToFile(LogLevel logLevel)
        {
            lock (_lockObject)
            {

                using (FileStream fs = new FileStream(Path.Combine(path, fileName), FileMode.Append, FileAccess.Write, FileShare.ReadWrite))
                {
                    using (StreamWriter sw = new StreamWriter(fs))
                    {
                        foreach (string msg in Msg.msgList)
                        {
                            sw.WriteLine(string.Format("{0} [{1}] {2}", DateTime.UtcNow.AddHours(08).ToString("yyyy/MM/dd HH:mm:ss"), logLevel, msg));
                        }
                    }
                }
            }
        }

    }
    public class LogBoth : ILog
    {
        public LogBoth()
        {
            Msg.msgList.Clear();
        }
        public void ExecWrite(LogLevel logLevel)
        {
            LogDB logDB = new LogDB();
            LogFile logFile = new LogFile();

            logDB.ExecWrite(logLevel);
            logFile.ExecWrite(logLevel);
        }

        public void WriteLog(string msg)
        {
            Msg.msgList.Add(msg);
        }
    }

Here are my observations:
LogLevel

LogLevel: Usually the industry calls this Severity Levels. I would suggest to try to follow these and do not re-invent the wheel.
In some cases it might worth to differentiate these two concepts:

Log Severity: The seriousness / weight / riskiness of a given event.
Log Level: Usually there is a particular order between the levels. The logger should log everything which is at a given level or above

For example: if you have this ordering: Debug > Information > Warning
and you set the level to Information then Debug logs will be omitted
from persistence.

Msg

Msg: Please try to avoid using abbreviations or short-forms in case of class names.
msgList: Please try to avoid Hungarian notation.
Msg.msgList: This seems quite clumsy and stuttering.

Please try to come up with better naming and encapsulation.

LogHelper

LogHelper: This is a LoggerFactory. If you have only just static members then mark your class static as well. By the way this is more like an abstract factory than a factory method.

Please read this related discussion.

throw new NullReferenceException: It is really rare to throw NullReferenceException manually. And fortunately this is not needed at all. You have at least two options:
single return statement:

ILog logger;
switch (logPlace)
{
    case "DB":
        logger = new LogDB();
        break;
    case "File":
        logger = new LogFile();
        break;
    case "Both":
        logger = new LogBoth();
        break;
    default:
        logger = new LogFile();
        break;
}

return logger;

or switch expression (in case of C# 8):
return logPlace switch
{
    "DB" => new LogDB(),
    "File" => new LogFile(),
    "Both" => new LogBoth(),
    _ => new LogFile(),
};

ILog

ILog: This should be ILogger to be precise.
void WriteLog(string msg): Usually here should the logger receive the log level / severity. Or it has several overloads with different severity levels, like LogInformation, LogWarning, LogError, etc.

Usually loggers can accept any arbitrary data (in a form of object) to give the ability to include/attach any useful information to the event.

void ExecWrite(LogLevel logLevel): Usually this is called Flush.

And usually there are some automation behind the scenes, which calls
this periodically or when the logs count reaches a given threshold.
Here you could consider to make this function asynchronous to take
advantage of async I/O (file and database writes are usually support
async I/O)

LogDb

LogDb: Yet again naming: DatabaseLogger would be way better in my opinion.
Msg.msgList.Clear(): This could be quite error-prone if you call it inside the ctor. Image the following situation: You have created a LogDb and wrote some log into it then you create a LogFile. Now all your logs are now gone.
LogDb'sExecWrite: It might make sense to use NotSupportedException if you have no willing to implement it. But in that case the whole class does not make too much sense.

LogFile

LogFile: FileLogger as a naming alternative.
LogFile' ExecWrite: This whole retry logic is quite naive. For example if the file path is invalid (pointing to a non-existing file or folder) then retrying the same action would not change anything.

With the current implementation of LogDb your catch block does
not make any sense.
Also you are swallowing the exception
without logging the fact that something went wrong.

WriteToFile: What if the file does not exists? Why do you use FileShare.ReadWrite? Why don't use async I/O?
DateTime.UtcNow.AddHours(08): This hard-coded time-zone handling makes your utility hard to maintain and even harder to make it reusable.

Also please bear in mind that all events will have the same timestamp
that are written to the file at the same time. A better approach
would be to add the timestamp to your log message at the time when
the consumer of your API calls the WriteLog method.

LogBoth

LogBoth: DatabaseAndFileLogger as a naming alternative.
LogBoth's ExecuteWrite: Here you have determined the ordering. If you have to change that then you have create a new class or create an overload.

If you would use async I/O then you could take advantage of concurrent persistence calls.
In the industry standard loggers there is no such thing like this. The consumer of your API can set up multiple sinks (persistence storage) in configuration file and the logger will take care of them.
It is quite challenging to implement it in the right way, because what should happen if either of them fails?


Text class for Pygame Failing to get a solution to my problem from other people, I got tired of waiting and wrote this (rather rushed) Text class to deal with the text problems I've been having in Pygame. I wanted the text to update itself, so that it would be unnecessary to keep track of its original position. After playing a lot with it, I found this solution:

class Text(object):

    def __init__(self, value, size, color,
                 left_orientation=False,
                 font=None,
                 x=0, y=0,
                 top=None, bottom=None, left=None, right=None,
                 centerx=None, centery=None):

        self._size = size
        self._color = color
        self._value = value
        self._font = pygame.font.Font(font, self._size)
        self.width, self.height = self._font.size(self._value)
        self.left_orientation = left_orientation

        self.image = self._create_surface()
        self.rect = self.image.get_rect()
        if x: self.rect.x = x
        if y: self.rect.y = y
        if top: self.rect.top = top
        if bottom: self.rect.bottom = bottom
        if left: self.rect.left = left
        if right: self.rect.right = right
        if centerx: self.rect.centerx = centerx
        if centery: self.rect.centery = centery

    def _create_surface(self):
        return self._font.render(self._value, True, self._color)

    def set_value(self, new_value):
        if new_value != self._value:
            self._value = new_value
            self.image = self._create_surface()

            new_rect = self.image.get_rect(x = self.rect.x, y = self.rect.y)
            if self.left_orientation:
                width_diff = new_rect.width - self.rect.width
                new_rect.x = self.rect.x - width_diff
            self.rect = new_rect

    def set_position(self, x_or_x_and_y, y=None):
        if y != None:
            self.rect.x = x_or_x_and_y
            self.rect.y = y
        else:
            self.rect.x = x_or_x_and_y[0]
            self.rect.y = x_or_x_and_y[1]

So, if a text is supposed to increase to the left, all that I have to do is initialize a Text object with the left_orientation parameter set to True and, whatever the rect is, it will update itself to remain at it's original position.

Is this a good solution? If not, what would be a better one?

This is copied from my answer to your question "What about my Pong game?".

The Text._size member is only used to create the font, so there is no need to store it in the instance.
Why not make Text a subclass of pygame.sprite.Sprite so that you can draw it using a sprite group?
Lots of code is repeated betwen Text.__init__ and Text.set_value. Why not have the former call the latter?
The Text.__init__ constructor takes many keyword arguments, which you then apply to self.rect like this:

if top: self.rect.top = top
...

The first problem is that the test if top: means that you can't set top to zero. You should write something like this:

if top is not None: self.rect.top = top

But it would be much simpler to use Python's ** keyword argument mechanism to take any number of keyword arguments, and pass them all to Surface.get_rect.

Applying all these improvements yields the following:

class Text(pygame.sprite.Sprite):
    def __init__(self, text, size, color, font=None, **kwargs):
        super(Text, self).__init__()
        self.color = color
        self.font = pygame.font.Font(font, size)
        self.kwargs = kwargs
        self.set(text)

    def set(self, text):
        self.image = self.font.render(str(text), 1, self.color)
        self.rect = self.image.get_rect(**self.kwargs)


My implementation of a for_each function that traverses a std::container by some step size integer N As my answer to Iterating over an odd (even) elements only in a range-based loop, I wrote this function, with the following driver program and output:

#include <array>
#include <vector>
#include <iterator>

// Forward iteration from begin to end by step size N.
template<typename Container, typename Function>
void for_each_by_n( Container&& cont, Function f, unsigned increment_by = 1) {
    if ( increment_by == 0 ) return; // must check this for no op

    using std::begin;
    auto it = begin(cont);

    using std::end;
    auto end_it = end(cont);

    while( it != end_it ) {
        f(*it);
        for ( unsigned n = 0; n < increment_by; ++n ) {
            if ( it == end_it ) return;
            ++it;
        }
    }
}

int main() {
    std::array<int,8> arr{ 0,1,2,3,4,5,6,7 };
    std::vector<double> vec{ 1.2, 1.5, 1.9, 2.5, 3.3, 3.7, 4.2, 4.8 };

    auto l = [](auto& v) { std::cout << v << ' '; };

    for_each_by_n(arr, l); std::cout << '\n';
    for_each_by_n(vec, l); std::cout << '\n';

    for_each_by_n(arr, l, 2); std::cout << '\n';
    for_each_by_n(arr, l, 4); std::cout << '\n';

    for_each_by_n(vec, l, 3); std::cout << '\n';
    for_each_by_n(vec, l, 5); std::cout << '\n';

    for_each_by_n(arr, l, 8); std::cout << '\n';
    for_each_by_n(vec, l, 8); std::cout << '\n';

    // sanity check to see if it doesn't go past end.
    for_each_by_n(arr, l, 9); std::cout << '\n';
    for_each_by_n(vec, l, 9); std::cout << '\n';

    return 0;
}

Output

 0 1 2 3 4 5 6 7
 1.2 1.5 1.9 2.5 3.3 3.7 4.2 4.8
 0 2 4 6
 0 4
 1.2 2.5 4.2
 1.2 3.7
 0
 1.2
 0
 1.2

I did my best to check for possible bugs, corner cases etc.

What I would like to know about my function above:

Does this follow modern C++ standards?
Is there any room for improvements?
Did I miss any possible bugs that I might have overlooked?
Would this be considered readable, reliable, generic, portable, cross-platform and reusable?
Do I have to worry about any const correctness, type deduction, cache misses and the like?
-Note-: I know the above function is not in a designated namespace; that is not of concern here. I can do that without hassle or trouble.

Let me know what you think; I'm looking forward to any and all feedback.

I would like to know ahead of time for I am thinking about adding a second unsigned integer parameter to this function. It would allow the user to choose the index location they want to use for their starting position. This parameter would default to 0.

We could use Container::size_type or std::size_t for the increment_by parameter, or (as standard algorithms like std::for_each_n seem to do) make it a template argument and use the same type while incrementing n.
The standard library algorithms take iterators rather than containers. You mention allowing users to specify an "index location they want to use for their starting position", which would be accomplished by passing an iterator range instead of calling begin and end.
I don't think the default increment_by argument value is useful. If we needed a step size of 1, we'd call std::for_each or use a range-based for loop.
std::for_each returns the function object (which can be helpful for something like summing values). We could do the same.
Follow the standard library conventions with naming template arguments (e.g. name the minimum required iterator type, make it clear that the function is a unary function).

Modified version:

template<class InputIt, class Size, class UnaryFunction>
UnaryFunction for_each_by_n(InputIt begin, InputIt end, Size step, UnaryFunction f) {

    if (step == 0)
        return f;

    while (begin != end)
    {
        f(*begin);

        for (Size n = 0; n != step; ++n)
        {
            if (begin == end)
                return f;

            ++begin;
        }
    }

    return f;
}

(edit: removed unnecessary std::move per Juho's comment).


Safely order a list of objects by DateTime I have several lists of various objects that need to be ordered by one of their properties that contains a string representation of DateTime, whose property also varies depending on the object.

The main issue with the lists is that the contained objects cannot be guaranteed to be valid DateTime representations and so simply ordering them (using LINQ) would throw exceptions if the property is null or an invalid format.

This means I have to use something like DateTime.TryParse. Here's the class method I wrote to do this:

static List<T> SafelyOrderListByDateTimeDescending<T>(List<T> list, string propertyName)
{
    DateTime value;
    PropertyInfo propInfo;
    string dateTimeString;
    foreach (T obj in list)
    {
        propInfo = obj.GetType().GetProperty(propertyName);
        dateTimeString = propInfo.GetValue(obj, null) as string;
        if (!DateTime.TryParse(dateTimeString, out value))
            propInfo.SetValue(obj, default(DateTime).ToString(), null);
    }

    return list.OrderByDescending(x => DateTime.Parse(x.GetType().GetProperty(propertyName).GetValue(x, null) as string)).ToList();
}

Considering there are many thousands of objects per list it's important that the method is not exceedingly inefficient. It seems to perform well in tests but I'd like some feedback on what could be improved or even if there's a better way.

I've tried some really neat ways of doing something similar but they all exclude the object if the value isn't successfully parsed whereas I still need the object.

You call this method SafelyOrderListByDateTimeDescending but there is nothing safe about it. I'd be really surprised if I found that a sorting function actually modified the objects I wanted to sort.

propInfo.SetValue(obj, default(DateTime).ToString(), null);

This shouldn't be there. You either need another method that sets default values for uninitialized properties so that the user knows his objects are modified or... you need another really safe approach. This means you need a special comparer that you can use with the OrderByDescending

class DateTimeComparer<T> : IComparer<T>
{
    private readonly PropertyInfo _property;

    public DateTimeComparer(string propertyName) => _property = typeof(T).GetProperty(propertyName);

    public int Compare(T left, T right)
    {
        return GetDateTimeOrDefault(left).CompareTo(GetDateTimeOrDefault(right));
    }

    private DateTime GetDateTimeOrDefault(T obj)
    {
        return
            DateTime.TryParse(_property.GetValue(obj) as string, out DateTime result)
            ? result
            : default(DateTime);
    }
}

It does not modifiy anything. Internally it either uses the value provided by the property or uses the default(DateTime). The constructor requires that you specify the name of the property to compare. (It's C# 7).

Example:

var result = values.OrderByDescending(x => new DateTimeComparer<YourType>("YourProperty"));


Functions for scanning command line options I know that getopts is available but I wanted to write my own as a way of improving my bash.  Below I show two function definitions and an example of the use of them in a bash script.  Finally I show the output of the script.

Please comment.

#!/bin/bash
# Produce a string variable which can be used by function isoption
# to give calling scripts an easy way to determine what options are
# are provided in its command line regardless of their order.
# Call this function as:
#
#   mygetopts "$@"
#
# Non-option arguments in the command line of the caller are
# passed back to the caller in array NONOPTARGS and can be used like this:
#
#   name=NONOPTARGS[0]
#
# NOTE: Option globbing is not allowed

function mygetopts ()
{
# Initialize a string variable which will be made to contain all
# option names in "$@".  Since this string will be used by function
# isoption a weird variable name is used to avoid collision with
# variable names in calling script since it cannot be made local.

    O1P2T3I4O5N6="-"    # O1P2T3I4O5N6 is avaiable to caller
    declare -i i=0      # i is local to function

    if [[ "$1" == "-h" || "$1" == "--help" ]]; then
        echo "Usage: $0 \"\$@\"" >&2
        return 1
    fi

    for arg in "$@" ; do    # arg will be each positional parameter
        if [[ ${arg:0:1} == - ]]    # if $arg begins with -
            # Pack all option names separated by -
            then O1P2T3I4O5N6=${O1P2T3I4O5N6}${arg:1}-

            else NONOPTARGS[i++]=$arg
        fi
        #echo "O1P2T3I4O5N6  =$O1P2T3I4O5N6"
        #echo "NONOPTARGS = ${NONOPTARGS[@]}"
    done
    return 0
}

function isoption ()
{

    if [[ $O1P2T3I4O5N6 =~ ^.*-${1}-.*$ ]]
        then return 0
        else return 1
    fi
}

#!/bin/bash
# Example of using functions mygetopts and isoption to provide easy
# access to all command line options as well as non-option arguments
# regardless of their order.
#
# To use these functions place the file MYFUNCTIONS in your $HOME
# directory and source it in your bash script before using the
# functions.  Alternately it can be sourced by your .bash_profile

set +vx         # Change + to - to enable tracing
echo
echo These $# command line arguments are provided:
echo "$@"

. ~/MYFUNCTIONS # source the function definitions
echo
echo These functions are defined:
compgen -A function # Show what functions are defined
echo
mygetopts "$@"  # Call mygetopts with all command line arguments

echo mygetopts returns this string with all options separated by -
echo O1P2T3I4O5N6 = $O1P2T3I4O5N6   # Show the string of all options

echo Examples of the use of isoption function:
if isoption f; then echo f found ; else echo f Not found; fi
if isoption z; then echo z found ; else echo z Not found; fi
if isoption abcd; then echo abcd found ; else echo abcd Not found; fi
if isoption abcde; then echo abcde found ; else echo abcde Not found; fi
echo
echo Examples of the accessing of non-option arguments:
inputfilename=${NONOPTARGS[0]}
outputfilename=${NONOPTARGS[1]}
echo inputfilename=$inputfilename
echo outputfilename=$outputfilename

***13:00:47 617 ~/work>testmygetopts -bde -f infile -abcd -x outfile -y

These 7 command line arguments are provided:

-bde -f infile -abcd -x outfile -y

These functions are defined:

isoption
mygetopts

mygetopts returns this string with all options separated by -

O1P2T3I4O5N6 = -bde-f-abcd-x-y-

Examples of the use of isoption function:

f found
z Not found
abcd found
abcde Not found

Examples of the accessing of non-option arguments:

inputfilename=infile
outputfilename=outfile

The bash code all looks reasonably fine to me.

The -h and --help output doesn't look right (it will print the actual parameters passed, but should print the list of allowable options/parameters?)

Options like --this-will-break won't work with --will or --break.

I'm not clear if parameters with spaces in them will work, as in: testmygetopts "this is one argument"

I think 'isoption' could be simplifed to just be the [[ ]] expression (since it will return 0 or 1 already).


Beginner FizzBuzz in Python This  is my first attempt at solving Fizzbuzz. I've always been pretty weak at algorithms, so I'm trying to improve. Specific feedback on performance would be appreciated.

def fizzBuzz(fizzNumber, buzzNumber):
  i = 0
  for i in range(1, 101):
      fizzBuzzValues = [i%fizzNumber, i%buzzNumber, i%(fizzNumber * buzzNumber)]
      if(0 in fizzBuzzValues):
          if(fizzBuzzValues[2] == 0):
              print("Fizzbuzz")
          elif(fizzBuzzValues[0] == 0):
              print("Fizz")
          elif(fizzBuzzValues[1] == 0):
              print("Buzz")
          else:
              break
      else:
          print(i)

fizzBuzz(3, 5)

Specific feedback on performance would be appreciated.

There are some things to improve performance-wise:

on every single iteration, you are calculating all the 3 remainders, even though you need a single one (in the best case)
the 0 in fizzBuzzValues check is not really needed (and the "in list" is O(n) in general, even though n=3 in this case, but you are doing that in a loop) - you can compare each of the remainders one by one
if you can calculate something before the loop - always do it - the fizzNumber * buzzNumber can be defined before the loop

Modified code:

def fizz_buzz_new(fizz_number, buzz_number):
    fizz_buzz_number = fizz_number * buzz_number
    for i in range(1, 101):
        if i % fizz_buzz_number == 0:
            print("Fizzbuzz")
        elif i % fizz_number == 0:
            print("Fizz")
        elif i % buzz_number == 0:
            print("Buzz")
        else:
            print(i)

Some timeit tests (replaced print() with yield to avoid seeing impact of printing to stdout):

$ ipython3 -i test.py

In [1]: %timeit for x in range(100): list(fizzBuzz(3, 5))
100 loops, best of 3: 3.52 ms per loop

In [2]: %timeit for x in range(100): list(fizz_buzz_new(3, 5))
100 loops, best of 3: 1.98 ms per loop


TypeScript fetch wrapper class with localStorage cache I'm learning TypeScript while doing my portfolio, and I wrote a class to make a fetch request to the API that holds my data.
It is working fine. I use localStorage to set a cache to avoid calling the API often as the data won't change.
I know I could improve this a lot, and haven't used many TypeScript features, so it would be great to have some pointers on how to improve this class generally, and also how to improve it by adding some more TypeScript features.
This is the class:
export interface PortfolioData {
  basics: Basics;
  projects?: Project[];
}

interface Cache {
  portfolio: PortfolioData;
  expiration: number;
}

class Portfolio {
  private url = 'https://gitconnected.com/v1/portfolio/mauriciorobayo';
  private cacheDurationInMilliseconds: number;

  constructor(cacheDurationInMinutes: number) {
    this.cacheDurationInMilliseconds = cacheDurationInMinutes * 60 * 1000;
  }

  private getCache(): Cache | undefined {
    const data = localStorage.getItem('portfolio');

    if (!data) {
      return;
    }

    const cache = JSON.parse(data);

    if (Date.now() > cache.expiration) {
      return;
    }

    return cache;
  }

  private setCache(data: Cache) {
    localStorage.setItem('portfolio', JSON.stringify(data));
  }

  async getPortfolio(): Promise<PortfolioData> {
    const cache = this.getCache();

    if (cache) {
      return cache.portfolio;
    }

    const response = await fetch(this.url);
    if (response.ok) {
      const portfolio = await response.json();

      this.setCache({
        portfolio,
        expiration: Date.now() + this.cacheDurationInMilliseconds,
      });

      return portfolio;
    }
    throw new Error(response.statusText);
  }
}

export default Portfolio;

The review will be focused on readability and testability.
Naming
I read (I think it was in Clean Code) that variables should not have a postfix like Data and Information and instead we should give things a name that they represent.
PortfolioData is actually a Portfolio while your current Portfolio is more a PortfolioRepository or PortfolioCollection.
Additionally I couldn't figure out, what Basic could mean and I looked up the json-representation of the gitconnected api:

"basics": {
   "name": "...",
   "picture": "https://avatars2.githubusercontent.com/u/2121481?v=4",
   "label": "...",
   "headline": "...",
   "summary": "...",
   "website": "...",
   "blog": "...",
   "yearsOfExperience": 2,
   "id": "...",
   "username": "...",
   "karma": 22,
   "email": "...",
   "region": "...",
   "location": {},
   "phone": "...",
   "followers": 63,
   "following": 94,
   "profiles": []
}

For me a more descriptive name would be User or Owner. A Portfolio has an Owner and a PortfolioRepository stores Portfolio-Objects:
export interface Portfolio {
  owner: Owner;
  projects?: Project[];
}

interface Cache {
  portfolio: Portfolio;
  expiration: number;
}

class PortfolioRepository { /* ... */ }

Concerns
The class Portfolio handles multiple concerns. There are two principles which can be named to emphasize the importance for separation:

Separation of Concerns (SoC)
Single Responsibility Principle (SRP)

The concerns are

fetching (await fetch(this.url))
caching (this.setCache(...))
querying (async getPortfolio(): Promise<PortfolioData>)

When we separate the concerns we could accomplish something like
interface PortfolioRepository {
    get(): Promise<Portfolio>;
}

class ApiPortfolioRepository implements PortfolioRepository {
    constructor(private cache: Cache,
                private api: API) {}

    async get(): Promise<Portfolio> {
        if (this.cache.containsPortfolio()) {
            return this.cache.get();
        }

        const portfolio = await this.api.fetch();
        this.cache.update(portfolio);

        return portfolio;
    }
}

This has multiple advantages. Beside the descriptive and shorter method body we can accomplish through the dependency injection via the constructor a better way to test the class.
Now the PortfolioRepository does not need to know about how caching works like the cacheDurationInMilliseconds and it does not need to know the http endpoint any more.
If we want to test the class we could simply mock the api (for exampe with mocha and chai):
import { assert } from "chai";

suite('API Portfolio Repository', () => {

    const cache = ...;
    const fakeAPI = new FakeAPI();

    test('when API returns no portfolio -> return undefined', () => {
        const repository = new ApiPortfolioRepository(cache, fakeAPI.withoutResponse);

        const portfolio = repository.get();

        assert.isUndefined(portfolio);
    });

    test('when API returns a portfolio -> return portfolio', () => {
        const repository = new ApiPortfolioRepository(cache, fakeAPI.withResponse);

        const portfolio = repository.get();

        assert.deepEqual(portfolio, fakeAPI.portfolio);
    });

})


Implement `Dispose` method in C# I have a class:

public class EmailSender: IDisposable
{
    private SmtpClient EmailSmtpClient { get; set; }
    private MailMessage Message { get; set; }

    public override void Dispose()
    {
         EmailSmtpClient.Dispose();
         Message.Dispose();
         GC.Collect();
    }
}

This is what I currently have implemented. What is the best way to release all the resources including the attachments (that are a part of MailMessage) immediately after the email is sent? Can anyone review my Dispose method and suggest some betterment? I will be thankful :)

I would suggest that you read these pieces from MSDN:

Basic Dispose Pattern,
 Implementing a Dispose Method

Some things that immediately stand out from your example:

Replace your GC.Collect() with GC.SuppressFinalize(this)
Check your resources aren't null before attempting to dispose them.
Check that your objects haven't already been disposed.

All of this is easily done by following the dispose pattern.


Thread-safe LRU Cache Implementation in Java I am trying to implement a thread-safe LRU cache algorithm, please review my code and would really appreciate some criticism or suggestion.
Some assumptions:

capacity will be larger than 0

Some questions I have:

I know it is better to encapsulate variables in a class and declare them private, but since DDLNode is simply a data class, is it acceptable to make the variables public ?

I am considering to extract the following block of code (inside put) into a function (something like addEntry(key, value)
DDLNode<K, V> node = new DDLNode<>(key, value);
addNode(node);
entries.put(key, node);

do you think it's neccesary? or the original code is self-explanatory enough?

Here is the code, Thanks in advance!!
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.locks.ReentrantLock;

interface CacheAlgorithm<K, V> {

    public V get(K key);

    public void put(K key, V value);
}

class DDLNode<K, V> {

    public V value;
    public K key;
    public DDLNode<K, V> prev;
    public DDLNode<K, V> next;

    public DDLNode() {}

    public DDLNode(K key, V value) {
        this.key = key;
        this.value = value;
    }
}

public class LRU<K, V> implements CacheAlgorithm<K, V> {

    private final ReentrantLock lock;
    private final int capacity;
    private final Map<K, DDLNode<K, V>> entries;
    private final DDLNode<K, V> head;
    private final DDLNode<K, V> tail;

    public LRU(int capacity) {
        this.capacity = capacity;
        lock = new ReentrantLock();
        head = new DDLNode<>();
        tail = new DDLNode<>();
        entries = new HashMap<>(capacity);
        head.next = tail;
        tail.prev = head;
    }

    @Override
    public V get(K key) {
        lock.lock();
        try {
            if (!entries.containsKey(key)) {
                return null;
            }
            DDLNode<K, V> node = entries.get(key);
            moveToMostRecent(node);
            return node.value;
        } finally {
            lock.unlock();
        }
    }

    @Override
    public void put(K key, V value) {
        lock.lock();
        try {
            if (entries.containsKey(key)) {
                DDLNode<K, V> node = entries.get(key);
                node.value = value;
                moveToMostRecent(node);
            } else {
                if (entries.size() == capacity) {
                    removeLeastRecentEntry();
                }
                DDLNode<K, V> node = new DDLNode<>(key, value);
                addNode(node);
                entries.put(key, node);
            }
        } finally {
            lock.unlock();
        }
    }

    private void removeLeastRecentEntry() {
        entries.remove(tail.prev.key);
        removeNode(tail.prev);
    }

    private void moveToMostRecent(DDLNode<K, V> node) {
        removeNode(node);
        addNode(node);
    }

    private void removeNode(DDLNode<K, V> node) {
        DDLNode<K, V> prev = node.prev;
        DDLNode<K, V> next = node.next;
        prev.next = next;
        next.prev = prev;
    }

    private void addNode(DDLNode<K, V> node) {
        DDLNode<K, V> headNext = head.next;
        head.next = node;
        node.prev = head;
        headNext.prev = node;
        node.next = headNext;
    }
}

```

I like the codestyle!
Code organization tips:

Its better to move DDLNode into LRU and make it static, because sole purpose of this class is to be used by LRU class.

If you are not going to expose DDLNode to outside world, then it make sense to make it also private. Keeping its fields as public is fine here.

If you want to expose DDLNode to outside world (for example, in iterator), you should think more thoroughly about field access. You can get inspiration from HashMap.Node class, for example: fields here have default access, so HashMap itself can manipulate them without using getter or setters. Also note, that there is no setKey() method for outside world, because if we change the key on the node it will break the map.

Maybe you just put all the classes into 1 file to make it easies for us to read , but if not - its bad practice to have more than 1 class/interface in 1 file.

Optimization tips:

In get() you are doing unnecessary .containsKey() call, because just .get(key) is enough: if key if present, it will find it; if not, then it will return null and you can then just return from method.

Unnecessary containsKey() call in put() as well: you can just check .get(key) == null instead.

head and tail will always occupy memory, event if map is empty. They could be removed, although then you should be more careful will null checks.


Retrieving a thumbnail photo for an ActiveDirectory account I have the following method and I would like to understand how it could be reorganized to be more unit-testable.

I think the section about binding to Active Directory could go to its own method, making this method more about either returning bytes or not.

public byte[] getThumbnail(String username) {
    SimpleBindRequest formInputBindRequest =
        new SimpleBindRequest(MASTER_USER_DN, MASTER_PASSWORD);
    try {
        LDAPConnection adConnection = getAdConnection();
        adConnection.bind(formInputBindRequest);

        SearchResult searchResults = adConnection.search(properties.getSearchBase(),
                SearchScope.SUB, "(sAMAccountName=" + username + ")", "thumbnailPhoto");
        if (searchResults.getEntryCount() > 0) {
            return searchResults.getSearchEntries()
                                .get(0)
                                .getAttributeValueBytes("thumbnailPhoto");
        }
        return null;
    }
    catch (Exception e)
    {
        return null;
    }
}

Without changing the code significantly what you need is a seam to insert a different LDAPConnection, and possibly properties. To create the seam for LDAPConnection you've got a couple of different options

Create a class that extends from the current to and overrides getAdConnection()
Constructor inject a LDAPConnectionFactory to the object and use that to replace getAdConnection()

Once you've done one of these you can inject your mock and make some assertions about how the search in being conducted and return a result structure.


First Scala FizzBuzz implementation Would love some feedback on this.  I'm coming from a Java background and kinda feel like I've just done exactly what I would do in Java. Is there a better way?

for (i <- 1 to 100) {
  if ( i % 3 == 0 && i% 5 == 0)
    println(i +" = FizzBuzz")
  else if (i % 3 == 0)
    println(i +" = Fizz")
  else if (i % 5 == 0)
    println(i +" = Buzz")
}

Or if you prefer match/case to if/else:

(1 until 100).map(i => (i % 3, i % 5) match {
  case (0, 0) => "FizzBuzz"
  case (0, _) => "Fizz"
  case (_, 0) => "Buzz"
  case _ => i
}).foreach(println)

Update:

So what we are doing here is taking list of numbers and mapping them first to tuples where on the left side is that number module 3 and on the right side modulo 5. Then we are matching those tuples to cases where both are zero, left is zero, right is zero or neither is zero.

Also the actual logic is in the map-block and side-effect of printing is in the foreach-block.


Find the number of working days between two dates inclusively The objective is to find the number of working days (days spanning from Monday to Friday) between two given dates inclusively.

using System;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            for (int i = 13; i <= 31; i++)
            {
                DateTime start = new DateTime(2014, 10, i);
                DateTime stop = new DateTime(2014, 10, 31);

                int totalWorkingDays = GetNumberOfWorkingDays(start, stop);

                Console.WriteLine("There are {1} working days from Oct {0}, 2014 to Oct 31, 2014.", i, totalWorkingDays);
            }
        }

        private static int GetNumberOfWorkingDays(DateTime start, DateTime stop)
        {
            TimeSpan interval = stop - start;

            int totalWeek = interval.Days / 7;
            int totalWorkingDays = 5 * totalWeek;

            int remainingDays = interval.Days % 7;

            for (int i = 0; i <= remainingDays; i++)
            {
                DayOfWeek test = (DayOfWeek)(((int)start.DayOfWeek + i) % 7);
                if (test >= DayOfWeek.Monday && test <= DayOfWeek.Friday)
                    totalWorkingDays++;
            }

            return totalWorkingDays;
        }
    }
}

Is there the simpler or better way to do so? Any comment and suggestion is welcome.

Personally, I think your approach is very complicated for what it should be trying to do: loop over all days between the start and the end date and count how many of those are not in the weekend.

Translating that sentence to code, gives me this:

private static int GetNumberOfWorkingDaysJeroen(DateTime start, DateTime stop)
{
    int days = 0;
    while(start <= stop)
    {
        if(start.DayOfWeek != DayOfWeek.Saturday && start.DayOfWeek != DayOfWeek.Sunday)
        {
            ++days;
        }
        start = start.AddDays(1);
    }
    return days;
}

I find this much easier to interpret than a bunch of modulos, casts and divisions.


Simple menu with descriptions If one menu-item is clicked then the description with the same data-filter attribute should be shown.

I added a class to the description elements to associate each one with the affiliated menu item.

What do you think?

$(".button").click(function() {
  $(".button").removeClass("active-filter");
  $(this).addClass("active-filter");

  var button = $(this).attr("data-filter");
  var active_element = $('.descr[data-filter="' + button + '"]');

  $(".descr").removeClass("active-element");
  active_element.addClass("active-element");
});
.element {
  display: inline-block;
  background-color: lightgreen;
  width: 220px;
}

.filter {
  display: inline-block;
  margin: 10px;
  background-color: orange;
}

.button {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.descr {
  display: block;
  max-height: 0;
  visibility: hidden;
  background-color: lightgrey;
  padding: 10px;
  position: absolute;
  top: 0;
  left: 0;
  right: 0;
  margin: 10px;
}

.data-element {
  height: 60px;
  display: block;
  background-color: orange;
  margin: 10px;
  position: relative;
}

.active-filter {
  background-color: yellow;
  display: block;
}

.active-element {
  background-color: yellow;
  max-height: 100%;
  visibility: visible;
}

data-filter 1

data-filter 2

data-element 1

data-element 2

One purpose for using jQuery is the ability to chain methods. You can first remove the "active-element" class from .d, then utilize .filter() with a single line passed as parameter within a template literal which removes the "active-filter" class from .b elements, filters and adds the same class to this element, and concludes by returning the .data() of the current element, which is the attribute value selector to match the corresponding .d element where the "active-element" class is added.

$(".b").click(function(e) {
  $(".d").removeClass("active-element")
  .filter(`[data-filter='${
    $(".b").removeClass("active-filter")
    .filter(this).addClass("active-filter")
    .data().filter
   }']`)
  .addClass("active-element");
});
.b {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.c {
  width: 60px;
  height: 60px;
  padding: 10px;
  margin: 10px;
  background-color: lightgrey;
  float: left;
}

.element {
  float: left;
  display: block;
}

.active-filter {
  background-color: pink;
}

.active-element {
  background-color: yellow;
}

data-filter

data-filter

data-element

data-element


Validate answers to online quiz I'm working on building a website to help people practice English. In this practice, the answer in the blank needs to be validated on hitting enter.

I'd like to know if there is a simpler/better way to write it since I need to use it for a lot of pages.

HTML:

There’s an energy here. At least, I think there is. Is it just that I know this city to be so? That there can be skiing a half hour away on the north shore mountains <input class="fillBlank" type="text" id="one" placeholder="your answer"> there is tennis in the city?

But then, <input class="fillBlank" type="text" id="two" placeholder="your answer"> my walk I took this photo below.

The city is also lush which means that it rains on <input class="fillBlank" type="text" id="three" placeholder="your answer">. Okay, maybe more than occasionally at this time of year.

CSS:

.fillBlank {
width: 100px;
border-radius: 0;
border: none;
background-color: #eff9f7;
padding: 3px;
text-align: center;
}

JS:

 $("#vancouver").ready(function() {
  $(".fillBlank").val("");
  });

// vancouver quiz answer one

var one = document.getElementById("one");
one.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valone(e);
}
});

function valone(e) {

var one = document.getElementById("one").value;

if (one === "while") {

document.getElementById("one").style.color = "green";
}

else {
document.getElementById("one").style.color = "red";
}
}

// vancouver quiz answer two

var two = document.getElementById("two");
two.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valtwo(e);
}
});

function valtwo(e) {

var two = document.getElementById("two").value;

if (two === "on") {

document.getElementById("two").style.color = "green";
}

else {
document.getElementById("two").style.color = "red";
}
}

// vancouver quiz answer three

var three = document.getElementById("three");
three.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valthree(e);
}
});

function valthree(e) {

var three = document.getElementById("three").value;

if (three === "occasion") {

document.getElementById("three").style.color = "green";
}

else {
document.getElementById("three").style.color = "red";
}
}

I can be rather quick with the CSS part, use indentation:

.example{
    oneTab: indented;
}

Round one

Then, you Javascript. Again, indentation. After that, I've removed several blank lines which had no purpose

Result:

 $("#vancouver").ready(function() {
    $(".fillBlank").val("");
});

// vancouver quiz answer one
var one = document.getElementById("one");
one.addEventListener("keydown", function (e) {
    if (e.keyCode === 13) {
        valone(e);
    }
});

function valone(e) {
    var one = document.getElementById("one").value;
    if (one === "while") {
        document.getElementById("one").style.color = "green";
    }
    else {
        document.getElementById("one").style.color = "red";
    }
}

// vancouver quiz answer two
var two = document.getElementById("two");
two.addEventListener("keydown", function (e) {
    if (e.keyCode === 13) {
        valtwo(e);
    }
});

/* repeat code for every question */

Round Two

Make use of the short if/else. Example below:

function valone(e) {
    var one = document.getElementById("one").value;
    //                                            Statement    ?  true   : false
    document.getElementById("one").style.color = one==="while" ? "green" : "red";
}
//---------------------------------------------------------------------
// And now save the 'one' element so we dont select it twice
function valone(e) {
    var one = document.getElementById("one");
    //                 Statement          ?  true   : false
    one.style.color = one.value==="while" ? "green" : "red";
}

Round Three

You repeat yourself for every question. This means that when you add one question, you have to update all of that. What if you add 15 questions? Imagine the huge amount of code that that would take!

So, time to fix that. All three of your functions do the same. They check an element for a specific value. They all get triggered on a keydown. Lets try to fix that to a single function:

// First select the elements we want to check the values of, you've allready classed those [fillBlank]:
$('.fillBlank').on('keydown', function(e){
    // At this point we have all the inputs on all keydown event, we only want enter:
    if (e.keyCode === 13) {
        // Now we have all the elements but only execute this on enter.
    }
});

Now comes the fun part. We want to be able to check ALL the inputs in one go, but give every input a UNIQUE value to check. For this we can use objects:

var correctAnswers = {
    one   : 'while',
    two   : 'on',
    three : 'occasion'
}
$('.fillBlank').on('keydown', function(e){ /* ... */ });

Neat little list like that right? Easy to expand aswell, just add one line et voila. We're now gonna add this to our code:

if (e.keyCode === 13) {
    // Now we have all the elements but only excecute this on enter.
    // Start checking the value for the input where the user pressed enter:
    // We can access the value via this.id because we named the input 'one', and there is a key 'one' in correctAnswers
    this.style.color = this.value=== correctAnswers[this.id] ? "green" : "red";
}

Well, thats it really. Now you can add plenty more items without building a million functions. The code below does the same thing as yours, but without hardcoding.

Result:

$("#vancouver").ready(function() {
    $(".fillBlank").val("");
});
// The correct answers:
var correctAnswers = {
    one   : 'while',
    two   : 'on',
    three : 'occasion'
}
// First select the elements we want to check the values of
$('.fillBlank').on('keydown', function(e){
    // At this point we have all the inputs on all keydown event, we only want enter:
    if (e.keyCode === 13) {
        this.style.color = this.value=== correctAnswers[this.id] ? "green" : "red";
    }
});

There are some extra things you can do to improve your code.

What if I type While. If you want to ignore uppercase, you can lowercase the this.value to always check the answers lowercased
You might want to add a check if this.id is an existing value of correctAnswers (hint: it's called .indexOf())
You might want to add/remove a class. CSS is for styling, not JS. This way, when you want to change the style of something correct/incorrect you do it in the stylesheet.
You might want to add the color names (or if you've updated the classnames) to a variable, e.g.: var outcomes = {right:'green', wrong: 'red'}; alert(outcomes.right);


Int implementation using linked lists This is the first piece of code I've written on my own since I started learning a couple of months ago. The code works and it takes an int as input and creates a linked list with each node representing a digit. I've added a function that sums two linked listed ints and returns a third list with the result. The rest of the code is also distributed in functions.

Before I move on, it would be greatly appreciated to know if I'm doing something wrong, if my code is properly structured or if I'm following a good/bad style.

#include <iostream>

using std::cout;
using std::cin;

struct intNode {
    int num;
    intNode *next;
};

int deallocateList(intNode *&s);
int intConversionHelper(int n);
int lenList(intNode *&s);
int sumListContent(intNode *&s);
intNode *sumLists(intNode *&s1, intNode *&s2);
int printNode(intNode *&s);
int intLen(int n);
intNode *intToList(int n);
void append(intNode *&s, int n);

int main() {
    int num1;
    int num2;
    cout << "Give me number 1: ";
    cin >> num1;

    cout << "Give me number 2: ";
    cin >> num2;

    intNode *list1 = intToList(num1);
    intNode *list2 = intToList(num2);
    cout << "\nPrinting ints as linked lists: \n";
    printNode(list1);
    printNode(list2);

    cout << "\nSum of the two lists (as a list too!)\n";
    intNode *sumList = sumLists(list1, list2);
    printNode(sumList);

    deallocateList(list1);
    deallocateList(list2);
    deallocateList(sumList);
}

int printNode(intNode *&s) {
    intNode *tmp = s;

    while (tmp != NULL) {
        cout << tmp->num;
        tmp = tmp->next;
    }
    cout << "\n";
}

int intLen(int n) {
    int len = 1;
    while (n > 9) {
        len++;
        n /= 10;
    }
    return len;
}

intNode *intToList(int n) {
    intNode *intList = NULL;
    int num;
    int dec = 10;

    num = n % 10;
    append(intList, num);
    int count = 1;
    while (count < intLen(n)) {
        num = n / dec % 10;
        dec = dec * 10;
        append(intList, num);
        count++;
    }
    return intList;
}

void append(intNode *&s, int n) {
    if (s == NULL) {
        s = new intNode;
        s->num = n;
        s->next = NULL;
    }
    else {
        intNode *nNode = new intNode;
        nNode->num = n;
        nNode->next = s;
        s = nNode;
    }
}

intNode *sumLists(intNode *&s1, intNode *&s2) {
    int sum1 = sumListContent(s1);
    int sum2 = sumListContent(s2);
    return intToList(sum1 + sum2);
}

int sumListContent(intNode *&s) {
    intNode *tmp = s;
    int sum = 0;
    int len = lenList(s);
    while (tmp != NULL) {
        sum += intConversionHelper(len) * tmp->num;
        len--;
        tmp = tmp->next;
    }
    return sum;
}

int lenList(intNode *&s) {
    intNode *tmp = s;
    int len = 0;
    while (tmp != NULL) {
        len++;
        tmp = tmp->next;
    }
    return len;
}

int intConversionHelper(int n) {
    float value = n;
    for (int i = 1; i < n; i++) {
    value = value * 10;
    }
    value = value / n;
    return value;
}

int deallocateList(intNode *&s) {
    intNode *tmp = s;
    while (s != NULL) {
        tmp = tmp->next;
        delete[] s;
        s = tmp;
    }
}

Here are some observations and suggestions that may help you improve your code.

Use objects

The program is written much more in the procedural style of C rather than in the object-oriented style of C++.  The list itself could be an object (as could the individual nodes), with most of the procedures as functions of that object.  This would reduce coupling and make the program easier to understand.  Not least, instead of requiring the user to explicitly call deallocateList(), the object's destructor could automatically be called.

Make sure all paths return a value

Both the printNode() and deallocateList() functions claim to return an int but neither of them actually do.  Either return a meaningful int value or change the function signature to void.

Match new and delete

As mentioned in comments to this question, new should be matched with delete and not delete[] which is a different operation.

Use better naming

The function named printNode is not well named.  A better name might be printList since it actually prints the entire list and not just a single node.

Avoid using raw pointers

In modern C++, we don't tend to use raw pointers as much as in C.  Instead, we either use smart pointers or objects.  In this case, I'd say that if you created objects, it would be better to return an object from sumLists() and intToList() rather than a pointer to a node.

Use const where practical

In almost all of your functions, one parameter is a reference to a pointer to an intNode which is the beginning node of a list.  In all cases, that list is unmodified by the function and so can (and should) be declared as const.

Understand references and pointers

Almost all of your functions take an intNode *& as an argument.  I would highly recommend that you use either pointers or references but not references to pointers.

Use nullptr rather than NULL

Modern C++ uses nullptr rather than NULL.  See this answer for why and how it's useful.

Minimize duplication of code

The current append looks like this:

void append(intNode *&s, int n) {
    if (s == NULL) {
        s = new intNode;
        s->num = n;
        s->next = NULL;
    }
    else {
        intNode *nNode = new intNode;
        nNode->num = n;
        nNode->next = s;
        s = nNode;
    }
}

However, most of the code is functionally identical whether s is NULL or not.  I'd rewrite it like this:

void append(intNode *&s, int n) {
    s = new intNode{n, s};
}

Avoid pointless numeric conversions

In the intConversionHelper() routine, we inexplicably have a conversion to float which is then converted back to an int when the value is returned.  Just use int and avoid the conversions.

Rethink your interface

Right now, only numbers that fit in an int are representable within this code, severely restricting the use of the list and functions such as sumLists.  Instead, consider adding digit at a time without converting each list to an int first.


MultiThreaded TCP Server with high CPU usage I wrote simple C# TCP Server application and my program use a lot of cup, i was wondering anyone can review my code and would be grateful for any hints and suggestion.

This is my code.

public class Program
    {
        static void Main(string[] args)
        {
            int _Port = Convert.ToInt32(System.Configuration.ConfigurationManager.AppSettings["TCPPort"]);
            string _IP = System.Configuration.ConfigurationManager.AppSettings["TCPIP"];
            TcpServer server = new TcpServer(_Port);
        }
    }

  public class TcpServer
    {
        private TcpListener _server;
        private Boolean _isRunning;
        private static int defaultMax = 8;
        public TcpServer(int port)
        {
            _server = new TcpListener(IPAddress.Any, port);
            _server.Start();
            _isRunning = true;
            LoopClients();
        }

public void LoopClients()
    {
        while (_isRunning)
        {

            TcpClient newClient = _server.AcceptTcpClient();
            SentFirstTimeData_New(newClient);
            Thread t = new Thread(new ParameterizedThreadStart(HandleClient));
            t.Start(newClient);
        }
    }

    public void HandleClient(object obj)
    {

        int requestCount = 0;
        byte[] bytesFrom = new byte[209666];
        string dataFromClient = null;
        Byte[] sendBytes = null;
        requestCount = 0;
        int byteread = 0;
        string respdata = string.Empty;
        byte[] buffer = new byte[4096];
        int numberOfBytesRead;

        TcpClient client = (TcpClient)obj;
        NetworkStream networkStream = client.GetStream();
        IPAddress address = ((IPEndPoint)client.Client.RemoteEndPoint).Address;

        Boolean bClientConnected = true;
        int id = Thread.CurrentThread.ManagedThreadId;
        while (bClientConnected)
        {

            try {
                if (networkStream.DataAvailable)
                {
                    byteread = networkStream.Read(bytesFrom, 0, (int)client.ReceiveBufferSize);
                    dataFromClient = Encoding.ASCII.GetString(bytesFrom).TrimEnd('\0');
                    System.Text.Encoding.ASCII.GetString(bytesFrom);

                    XmlDocument doc = new XmlDocument();
                    string xml = Encoding.UTF8.GetString(bytesFrom).TrimEnd('\0');
                    doc.LoadXml(xml);
                    XElement incc = XElement.Load(new XmlNodeReader(doc));
                    respdata = AysncTCPIncomeString(incc);

                    sendBytes = Encoding.ASCII.GetBytes(respdata);
                    networkStream.Write(sendBytes, 0, sendBytes.Length);
                    networkStream.Flush();

                }

            }
            catch (Exception ex)
            {
                Helper.WriteLog("Exception=" + ex.Message);
            }

        }
    }

    public void SentFirstTimeData_New(TcpClient tc)
    {
        try
        {
            int threadid = Thread.CurrentThread.ManagedThreadId;
            string Msg = HLStatusResponse.FirstTimeStatusSuccessfulResponse("STATUS", "1");
            NetworkStream _NetworkStream = tc.GetStream();
            if (_NetworkStream != null)
            {
                byte[] data = Encoding.ASCII.GetBytes(Msg);
                _NetworkStream.Write(data, 0, data.Length);

                Helper.WriteLog("ThreadId=" + threadid.ToString() + ";SentFirstTimeData :\t " + Msg);
            }

        }
        catch (Exception ex)
        {
            Helper.WriteLog("Sent Though Tcp Eror :" + ex.ToString());

        }
    }

}

Big picture, you probably don't want to allocate and start a new thread to handle each incoming connection.  .Net has a very good ThreadPool implementation that scales up to as many threads as your machine can support without creating so many that performance suffers.

That said, it looks like your particular problem is that threads are never allowed to complete.  bClientConnected is the control variable for your main while loop, and it is never set to false, even if an exception is thrown.  You should always be suspicious of a catch that doesn't rethrow or at least exit the current function.


Building an interval-based grid I have an interesting problem to solve and despite having a running solution, I was wondering where I could improve it.

In a range from 1 to 1000, I want to create an array with different increments according to the current interval.
So, from 1.0 to 2.0, the increment should be 0.01, from 2.0 to 4.0 it should be 0.02, and so on. There are many intervals. The final result yields an array with 350 fields.

I've come up with a super basic, easy to implement solution using ifs. So it goes something like:

// In one file I have:
const grid = new Grid().buildGrid();
console.log(grid); // And expect a grid which respects the rules defined in the below class.

// Grid Class:
class Grid {
    constructor() {
        this.array = [];
    }

buildGrid() {
    _createKeysWithTicks(this.array);
}

_createKeysWithTicks(array){
    let i = 1.0;
    while (i < 1000){
        i = parseFloat((i + findStepSize(i)).toFixed(2));
            array.push(i);
    }
    return array;
}

findStepSize(number) {
    let res;
    if (number >= 1.0 && number < 2.0) {
        res = 0.01;
    } else if (number >= 2.0 && number < 4.0) {
        res = 0.02;
    } else if (number >= 4.0 && number < 5.0) {
        res = 0.1;
    } else if (number >= 5.0 && number < 10.0) {
        res = 0.5;
    } else if (number >= 10.0 && number < 50.0) {
        res = 1.0;
    } else if (number >= 50.0 && number < 100.0) {
        res = 10;
    }

    return res;
}

However, this is not elegant, at all because I get numerous if statements. I could swap for a switch statement, thus reducing the number of comparisons, which is nice. But does it improve performance?

The suggestion is:

findStepSize(i) {
    let res;
    switch (number):
        case(number >= 1.0 && number < 2.0) {
            res = 0.01;
            break;
        case(number >= 2.0 && number < 4.0) {
            res = 0.02;
            break;
        case(number >= 4.0 && number < 5.0) {
            res = 0.1;
            break;
        case(number >= 5.0 && number < 10.0) {
            res = 0.5;
            break;
        case(number >= 10.0 && number < 50.0) {
            res = 1.0;
            break;
        case(number >= 50.0 && number < 100.0) {
            res = 10;
            break;
    }
    return res;
}

The increments are not linear, otherwise the solution could be different.

How would you optimize or clean this code?

Note: I understand and respect the rules of this community and I appreciate your feedback. Unfortunately, I cannot disclose much more details. If this is still not sufficient for you to think of how would you get rid of those If statements, I can't help much more because I can't simply provide more details. In fact, the data above is random and it's not the use case.

You can take advantage of the fact that you're iterating in increasing order, i.e. the trivial fact that if a number is greater than n, then it's also greater than any number below n; thus you can just keep track of the latest threshold you passed and only check if you've reached the next one.

let thresholds = [2, 4, 5, 10, 50, 100, 1000];
let increments = [0.01, 0.02, 0.1, 0.5, 1, 10];
let p = 0;

function createKeysWithTicks(array) {
    let i = 1.0;
    while (i < 1000) {
        array.push(Number(i.toFixed(2)));
        i += increments[p];
        if (i >= thresholds[p]) {
            p++;
        }
    }
 }


Windows autoclicker with a console interface I have recently started coding in c++ and have made a simple console application. When executed the program asks the user to position their cursor wherever they want continuous repeating clicks, after 7 seconds it then asks the user to set an interval for the clicks. Since I am fairly new I am wondering if there is anything I could have done better to optimise the performance of the code or any other tips and tricks I can use, much appreciated

my code is (main.cpp):

#include <iostream>
#include <Windows.h>

using namespace std;

void leftClick(double mouse_x, double mouse_y){
        INPUT input;
        input.type=INPUT_MOUSE;
        input.mi.dx=(mouse_x * 65536) / GetSystemMetrics(SM_CXSCREEN) + 1;
        input.mi.dy=(mouse_y * 65536) / GetSystemMetrics(SM_CYSCREEN) + 1;
        input.mi.dwFlags=(MOUSEEVENTF_ABSOLUTE|MOUSEEVENTF_MOVE|MOUSEEVENTF_LEFTDOWN|MOUSEEVENTF_LEFTUP);
        input.mi.mouseData=0;
        input.mi.time=0;
        SendInput(1,&input,sizeof(INPUT));
}

int main(){
    cout << "Place the cursor where you want to click..." << endl;
    Sleep(7000);

    POINT p;
    GetCursorPos(&p);

    cout << "Set the interval to click: ";

    int cinterval {0};
    cin >> cinterval;

    cout << "Current Interval: " << cinterval * 1000 << endl;

    while(true){
        leftClick(p.x, p.y);
        Sleep(cinterval * 1000);
    }

     return 0;
}

Here are some suggestions you can use to improve your code.

Formatting

Insert a space before a { that starts a compound statement and does not go on a separate line; e.g., instead of int main(){, write

int main() {

or

int main()
{

Place a space after a comma and after a control word (while, etc.):         SendInput(1, &input, sizeof(INPUT)) and while (true) {.

Place two spaces around the = operator.

The indentation level of the leftClick function should be reduced to 4 spaces to keep consistent with main.

Naming

In C++, functions are usually named in snake_case instead of camelCase.  This is subjective though.

Standard library usage

Please try to avoid using namespace std; because it is considered bad practice.  The contents of the C++ standard library are put in a special namespace to avoid polluting the global namespace and causing name clashes.  The std namespace is not intended for use with using directives (in general).  using namespace std; forces you to avoid standard names and invalidates the purpose of the std namespace.  See Why is “using namespace std;” considered bad practice?.

std::endl is generally inferior to \n, but it is necessary in this case because of flushing requirements.  Good.

Currently, your code isn't clear in terms of unit of time.  Is the interval intended to be in seconds, milliseconds, or microseconds?  The standard <chrono> library solves this problem.  And instead of Sleep, use std::this_thread::sleep_for from <thread>, which plays well with <chrono>:

using namespace std::chrono_literals;

std::cout << "Place the cursor where you want to click ..." << std::endl;
std::this_thread::sleep_for(7s); // NB: valid syntax!

Indicate your presumed unit of time:

cout << "Set the interval to click (ms): ";

int cinterval {0};
cin >> cinterval;

// ...
std::this_thread::sleep_for(std::chrono::milliseconds{cinterval});


Making a string by reading down a matrix Suppose I have a string with a length always a multiple of four:

'CodeReviewSE'

My goal is to create a permutation. First, I arrange the string into a matrix (where each row has a length of 4):

    C  o  d  e
    R  e  v  i
    e  w  S  E

And then I simply read down, starting from the top-left corner. I get four segments: CRe, oew, dvs and eiE. I concatenate them, to get the following string:

'CReoewdvSeiE'

Reversing the operation is just reversing the steps, kinda. It follows the same steps, except for row length are len(text) / 4, and we read down again, to look like this:

    C  R  e
    o  e  w
    d  v  S
    e  i  E

I've implemented the following code in Python:

import collections

def permute(text):
    # type: (str) -> str

    rows = collections.deque()
    output = ''

    # Create matrix
    for i in xrange(0, len(text), 4):
        rows.append(text[i:i+4])

    # Read matrix downwards
    for counter in xrange(4):
        for row in rows:
            output += row[counter]

    return output

def unpermute(text):
    # type: (str) -> str

    rows = collections.deque()
    output = ''
    length = len(text) / 4

    # Create matrix
    for i in xrange(0, len(text), length):
        rows.append(text[i:i+length])

    # Now read downwards
    for counter in xrange(length):
        for row in rows:
            output += row[counter]

    return output

I also ran a performance tests using the timeit module, using random strings of different lengths (where n is the length):

n = 32                                    Best:       |       Avg
Permute: (10,000 loops, best of 10):     0.06563            0.07080
Unpermute: (10,000 loops, best of 10):   0.06337            0.06655

n = 64                                    Best:       |       Avg
Permute: (10,000 loops, best of 10):     0.11592            0.11939
Unpermute: (10,000 loops, best of 10):   0.10358            0.10530

n = 256                                   Best:       |       Avg
Permute: (10,000 loops, best of 10):     0.43404            0.44310
Unpermute: (10,000 loops, best of 10):   0.37202            0.38102

n = 1024                                  Best:       |       Avg
Permute: (10,000 loops, best of 10):     1.82779            1.85647
Unpermute: (10,000 loops, best of 10):   1.72195            1.78491

Most important comment: permute is just unpermute with length = 4.

There seems to be no reason to favor collections.deque over a normal list here. It is only an advantage of you do a lot of queue.popleft() calls.

For the creation of rows you can just use a list comprehension:

rows = (text[i:i+4] for i in xrange(0, len(text), 4))

I would use a generator expression here to save memory

You could simplify your un-/permute to: something like:

def permute(text, n=4, reverse=False):
    n = len(text)/n if reverse else n
    matrix = (text[i:i+n] for i in range(0, len(text), n))
    matrix_t = zip(*matrix)
    return "".join("".join(x) for x in matrix_t)

where (text[i:i+n] for i in range(0, len(text), n)) makes a generator object that is like your rows, zip(*list) transposes a list of lists and the two joins first join each row and then the rows.

This will not even create a lot of overhead because they are all generator objects (and therefore it is not one possibly big list being read into memory).

You could even inline all of it:

def permute(text, n):
    return "".join("".join(x) for x in zip(*(text[i:i+n] for i in range(0, len(text), n))))


Python exception handler to recommend package Consider this python 3 snippet, where when an import exception is raised, a string 'e' is created that says: "No module named ' packageName'".

The goal is to simply say on exit, that the import error can perhaps be solved by installing a package of the same name. I came up with this, and it works but I'd like to make it better, if anyone has some tips.

except ImportError as e:
     e = e.replace("No module named '", "")
     e = e.replace("'", "")
     sys.exit("'Error: Try sudo pip install %s'" % e)

Example output:

  Error: Try sudo pip install Crypto

An ImportError instance doesn't have a replace method.

You can use e.name instead of messy string replacements.

Instead of '%s' % x, it's recommended to use '{}'.format(x).

The example output is not what you posted, but actually this:

  'Error: Try sudo pip install Crypto'

That is, it's surrounded by single quotes, because you wrote as "'some text'".
Probably you want to remove those single quotes inside the string.

What does the message even mean?

This would make a lot more sense:

  Error: could not import 'Crypto'. Try installing with: pip install --user Crypto

Last, but most importantly, please don't recommend to install Python packages system-wide. This is a more sensible recommendation:

pip install --user the_package

This doesn't need sudo, as it will install the package in the user's home directory, not system-wide.

Putting it together:

try:
    import somename
except ImportError as e:
    sys.exit("Error: could not import '{0}'. Try installing with: pip install --user {0}".format(e.name))


Factory to construct a FineTime out of many kinds of objects Any suggestions to help refactor this method? I thought maybe some kind of factory to allocate the right type?

public static FineTime getFineTime(Object o) {
    if (o == null) {
        return null;
    }
    //FineTime
    if (o instanceof FineTime) {
        return (FineTime ) o;
    }
    //Long
    if (o instanceof Long) {
        return new FineTime((Long ) o);
    }
    //Int
    if (o instanceof Integer) {
        return new FineTime((Integer ) o);
    }
    //Date
    if (o instanceof Date) {
        try {
            return DateConverter.dateToFineTime((Date ) o);
        } catch (IllegalArgumentException iae) {
            //Try another converter
            LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(Date) failed", iae);
        }
    }
    //String
    if (o instanceof String) {
        String date = (String ) o;
        //TAI
        try {
            return _stfFormatTai.parse(date);
        } catch (IllegalArgumentException iae) {
            //Try another converter
            LOGGER.log(Level.FINE, "_stfFormatTai.parse(String) failed", iae);
        }
        //UTC
        try {
            return _stfFormatUtc.parse(date);
        } catch (IllegalArgumentException iae) {
            //Try another converter
            LOGGER.log(Level.FINE, "_stfFormatUtc.parse(String) failed", iae);
        }
        //dd-MMM-yyyy HH:mm:ss
        try {
            return DateConverter.dateToFineTime(_sdfFormat1.parse(date));
        } catch (ParseException pe) {
            //Try another converter
            LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat1.parse(String) failed", pe);
        } catch (IllegalArgumentException iae) {
            //Try another converter
            LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat1.parse(String) failed", iae);
        }
        //dd-MM-yyyy HH:mm:ss
        try {
            return DateConverter.dateToFineTime(_sdfFormat2.parse(date));
        } catch (ParseException pe) {
            //Try another coverter
            LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat2.parse(String) failed", pe);
        } catch (IllegalArgumentException iae) {
            //Try another converter
            LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat2.parse(String) failed", iae);
        }
    }
    LOGGER.info("Could not convert " + StringUtil.asString(o) + " to FineTime");
    //No converter found, return 'null'
    return null;
}

Overloading!

public static FineTime getFineTime(Long o){
    if(o == null){ return null; }
    return new FineTime(o);
}

public static FineTime getFineTime(Integer o){
    if(o == null){ return null; }
    return new FineTime(o);
}

public static FineTime getFineTime(FineTime o){
    return o;
}

public static FineTime getFineTime(Date o){
    if(o == null){ return null; }
    try {
        return DateConverter.dateToFineTime(o);
    } catch (IllegalArgumentException iae) {
        //Try another converter
        LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(Date) failed", iae);
        return null;
    }
}

public static FineTime getFineTime(String date){
    if(date == null){ return null; }
    //TAI
    try {
        return _stfFormatTai.parse(date);
    } catch (IllegalArgumentException iae) {
        //Try another converter
        LOGGER.log(Level.FINE, "_stfFormatTai.parse(String) failed", iae);
    }
    //UTC
    try {
        return _stfFormatUtc.parse(date);
    } catch (IllegalArgumentException iae) {
        //Try another converter
        LOGGER.log(Level.FINE, "_stfFormatUtc.parse(String) failed", iae);
    }
    //dd-MMM-yyyy HH:mm:ss
    try {
        return DateConverter.dateToFineTime(_sdfFormat1.parse(date));
    } catch (ParseException pe) {
        //Try another converter
        LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat1.parse(String) failed", pe);
    } catch (IllegalArgumentException iae) {
        //Try another converter
        LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat1.parse(String) failed", iae);
    }
    //dd-MM-yyyy HH:mm:ss
    try {
        return DateConverter.dateToFineTime(_sdfFormat2.parse(date));
    } catch (ParseException pe) {
        //Try another coverter
        LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat2.parse(String) failed", pe);
    } catch (IllegalArgumentException iae) {
        //Try another converter
        LOGGER.log(Level.FINE, "DateConverter.dateToFineTime(_sdfFormat2.parse(String) failed", iae);
    }
    return null;
}

And a default implementation

public static FineTime getFineTime(Object o){
    LOGGER.info("Could not convert " + StringUtil.asString(o) + " to FineTime");
    //No converter found, return 'null'
    return null;
}

And gone are most of your troubles.

The problem in this case, however, is that you do need to know what type your object has. You cannot "just" insert an object and get a value out. To do so anyway seems to imply bad design.

If you wanted to go that direction anyway, consider making a map of Class, Converter of sorts.

Something else you could do is leave the method you have right now as is, and just wrap your String and Date conversions in a separate function. That would significantly simplify the method as well, boiling down to "if integer, finetime via integer, if long, finetime via long, if string, finetime via string, if date, finetime via date, else log error and return null".