JavaScript path parsing I was attempting to do some contribution to the underscore-contrib library, and I ended up writing a function that will parse a JavaScript path to later on expose through an API if the path exists or to get the value.

However, a reasonable concern around it is the complexity. (Discussion in the repository, if you're interested.)

I did review other approaches to the problem, but most of them seem to leave valid complex property names behind, or some even are tied to actually retrieving the value at that point.

Can this be made simpler?

// Will take a path like 'element[0][1].subElement["Hey!.What?"]["[hey]"]'
// and return ["element", "0", "1", "subElement", "Hey!.What?", "[hey]"]
function parseJavaScriptPathIntoKeyNames(javascriptPath) {
  var parts = [];
  var terminatorExpected = null;
  var insideIndexer = false;
  var currentPart = "";

  function flushCurrentPart() {
    if (currentPart.length > 0) {
      parts.push(currentPart);
      currentPart = "";
    }
  }

  for (var i = 0; i < javascriptPath.length; i++) {
    var currentChar = javascriptPath[i];
    switch (currentChar) {
    case "[":
      if (!terminatorExpected) {
        flushCurrentPart();

        terminatorExpected = ']';
        insideIndexer = true;
      } else {
        currentPart += currentChar;
      }
      break;
    case "]":
      if (terminatorExpected === "]") {
        flushCurrentPart();

        terminatorExpected = null;
        insideIndexer = false;
      } else {
        currentPart += currentChar;
      }
      break;
    case ".":
      if (!terminatorExpected) {
        flushCurrentPart();
      } else {
        currentPart += currentChar;
      }
      break;
    case "\'":
      if (!terminatorExpected || terminatorExpected === "]") {
        terminatorExpected = "\'";
      } else if (terminatorExpected === "\'" && insideIndexer) {
        terminatorExpected = ']';
      } else if (terminatorExpected === "\'" && !insideIndexer) {
        flushCurrentPart();

        terminatorExpected = null;
      } else {
        currentPart += currentChar;
      }
      break;
    case "\"":
      if (!terminatorExpected || terminatorExpected === "]") {
        terminatorExpected = "\"";
      } else if (terminatorExpected === "\"" && insideIndexer) {
        terminatorExpected = ']';
      } else if (terminatorExpected === "\"" && !insideIndexer) {
        flushCurrentPart();

        terminatorExpected = null;
      } else {
        currentPart += currentChar;
      }
      break;
    default:
      currentPart += currentChar;
    } // switch (currentChar)
  } // for

  flushCurrentPart();

  return parts;
}

It would be a good idea to reimplement the whole thing using a regular expression.  Some uninitiated programmers despise or fear regular expressions, but in this case, avoiding them just means that you are giving up a language feature that is designed to solve just this kind of problem, forcing you to reinvent the wheel.

function parseJavaScriptPathIntoKeyNames(path) {
    /**
     * Repeatedly capture either:
     * - a bracketed expression, discarding optional matching quotes inside, or
     * - an unbracketed expression, delimited by a dot or a bracket.
     */
    var re = /\[("|'|)(.*?)\1\]|([^.\[\]]+)/g;

    var elements = [];
    var result;
    while ((result = re.exec(path)) !== null) {
        elements.push(result[2] || result[3]);
    }
    return elements;
}

I can't say for sure that this version behaves exactly the same as the original, especially in the face of illegal input.  However, the reduction in code makes it a much more worthwhile approach, and you can work out the details.


Javascript Reddit Bot Some days ago I've written JavaScript bot for reddit. All it does is replacing not well-formatted gfycat URLs. I would love to hear feedback about this code!

const chalk = require('chalk')
const Snoowrap = require('snoowrap')
const async = require('async')

console.log(chalk.cyan.bold('GfycatDetailsConvert is booting up...'))

let lastChecked

require('dotenv').config()

if (typeof process.env.REDDIT_USERAGENT === 'undefined') {
  throw new Error('Please fill your .env file. For more information, go here: https://github.com/ImJustToNy/GfycatDetailsConvert')
}

const r = new Snoowrap({
  userAgent: process.env.REDDIT_USERAGENT,
  clientId: process.env.REDDIT_CLIENTID,
  clientSecret: process.env.REDDIT_CLIENTSECRET,
  username: process.env.REDDIT_USERNAME,
  password: process.env.REDDIT_PASSWORD
})

r.config({
  requestDelay: 2000,
  continueAfterRatelimitError: true,
  maxRetryAttempts: 5,
  debug: process.env.NODE_ENV != 'production'
})

setInterval(() => {
  r.getNew('all', {
    before: lastChecked,
    show: 'all',
    amount: 1000
  }).then(posts => {
    if (posts.length > 0) {
      lastChecked = posts[0].name

      async.every(posts, (post, callback) => {
        if (post.domain === 'gfycat.com' && /(\/[a-z][a-z])?\/gifs\/detail/g.test(post.url)) {
          post.fetch().comments.map(comment => comment.author.name).then(participants => {
            callback(null, true)
            if (!participants.includes(process.env.REDDIT_USERNAME)) {
              console.log(chalk.red(chalk.bold('Found new post: ') + post.title + ' [/r/' + post.subreddit.display_name + ']'))

              post.reply('[Proper Gfycat URL](' + post.url.replace(/(\/[a-z][a-z])?\/gifs\/detail/g, '') + ') \n\n' + '^^I\'m ^^just ^^a ^^bot, ^^bleep, ^^bloop. [^^[Why?]](https://gist.github.com/ImJustToNy/cb3457e36f22123eb93864f0af639da3) [^^[Source ^^code]](https://github.com/ImJustToNy/GfycatDetailsConvert)')
            }
          })
        }
      })
    }
  })
}, 5000)

Overall, it gets the job done, but your code (especially the area I'd refer to as "pyramid code"), could use a little more organization and clarity.

Your if statement could be a little more concise, and, at the same time, more canonical about what it's actually there for:

if (!('REDDIT_USERAGENT' in process.env)) {
  ...
}

You can write concise code without using cryptic variable names. Change r to something more meaningful like robot or even just bot.

One way to resolve pyramid code is to look for places where your logic can be decoupled or flattened. While it's possible to continue to use the async dependency in order to achieve this, it's probably much easier to just use async/await since you're already using promises anyway. This will also give you an opportunity to get rid of your global variable lastChecked:

function matchesRequirements (post) {
  return post.domain === 'gfycat.com' && /(\/[a-z][a-z])?\/gifs\/detail/g.test(post.url)
}

async function tryReply (post) {
  const { comments, subreddit, title, url } = await post.fetch()
  const participants = comments.map(comment => comment.author.name)

  if (!participants.includes(process.env.REDDIT_USERNAME)) {
    console.log(chalk.red(`${chalk.bold('Found new post:')} ${title} [/r/${subreddit.display_name}]`))

    const proper = url.replace(/(\/[a-z][a-z])?\/gifs\/detail/g, '')
    const reason = 'https://gist.github.com/ImJustToNy/cb3457e36f22123eb93864f0af639da3'
    const source = 'https://github.com/ImJustToNy/GfycatDetailsConvert'

    const text = `[Proper Gfycat URL](${proper}) \n\n^^I'm ^^just ^^a ^^bot, ^^bleep, ^^bloop. [^^[Why?]](${reason}) [^^[Source ^^code]](${source})`

    return post.reply(text)
  }
}

async function pollNewPosts () {
  let lastChecked

  while (true) {
    const posts = await bot.getNew('all', {
      before: lastChecked,
      show: 'all',
      amount: 1000
    })

    if (posts.length > 0) {
      lastChecked = posts[0].name
    }

    await Promise.all(posts
      .filter(matchesRequirements)
      .map(tryReply)
    )

    await new Promise(resolve => setTimeout(resolve, 5000))
  }
}

pollNewPosts()

One of the things in this rewrite to flatten your pyramid code, I changed the order in which .fetch() is called, because the way your code accessed the comments property relied on the usage of a proxy get trap in order to work properly, which is both unnecessary and inefficient.

Lastly, I cleaned up your generated text to break it up into a few lines, otherwise it's quite unreadable. You might consider storing that text to an external template file using a minimal template engine (possibly written yourself just for this specific case) in order to avoid hard-coding it in here.


Given a string containing just parentheses, determine if the input string is valid The task
is taken from leetcode

  Given a string containing just the characters '(', ')', '{', '}', '['
  and ']', determine if the input string is valid.

  An input string is valid if:

  Open brackets must be closed by the same type of brackets.
  Open brackets must be closed in the correct order.
  Note that an empty string is also considered valid.

  Example 1:

Input: "()"

Output: true

  Example 2:

Input: "()[]{}"

Output: true

  Example 3:

Input: "(]"

Output: false

  Example 4:

Input: "([)]"

Output: false

  Example 5:

Input: "{[]}"

Output: true

My functional solution

/**
 * @param {string} s
 * @return {boolean}
 */
var isValid = s => {
  const par = {
      "(": ")",
      "{": "}",
      "[": "]",
    };
  const rep = [];
  return [...s].every(x => par[x]
                      ? rep.push(par[x])
                      : rep.pop() === x) && !rep.length;
};

My imperative soltion

/**
 * @param {string} s
 * @return {boolean}
 */
var isValid2 = s => {
  const par = {
      "(": ")",
      "{": "}",
      "[": "]",
    };
  const rep = [];
  for (let i = 0; i < s.length; i++) {
    if (par[s[i]]) {
      rep.push(par[s[i]]);
    } else {
      if (rep.pop() !== s[i]) {
        return false;
      }
    }
  }
  return rep.length === 0;
};

The code looks quite succinct and sufficient. The only suggestions I would offer are minor tweaks:

for the imperative solution, a for...of loop could be used to eliminate the need to do bookkeeping on the counter variable and use it to index into the string to get each character:

for(const c of s) {
  if (par[c]) {
    rep.push(par[c]);
  } else {
  if (rep.pop() !== c) {
    return false;
  }
}

for the functional solution, the callback function could be declared on a previous line, and then the return line can be reduced to a single line:

const pushOrPopMatch = x => par[x] ? rep.push(par[x]) : rep.pop() === x
return [...s].every(pushOrPopMatch) && !rep.length;

That line defining pushOrPopMatch() is 73 characters long, which some might argue is too long for a line, given it would be inside a function and indented at least two spaces, so it may not be ideal


Potentially superfluous iOS interface system I have written an interface system for iOS that allows interface elements to be moved around on the screen as actors move around on stage. It can operate within a ViewController environment. It relies on messaging through the Notification Centre (obviously not LocalNotification).

The central object is the ARKView object. It has 'ARKState objects associated with it that contain information about where it should be (along with colour, alpha, transform, etc.) at each 'scene'.

My question:

I have Googled many moons to find a system which does this already. It turns out that having hundreds of objects on screen all sending each other notifications is not very efficient. Perhaps I can do it with delegates? I need a system in which I can precisely specify the position and properties of every element upon pressing a button or other stimulus.

Code

ARKView/receiveNotification:

- (void)receiveNotification:(NSNotification *)notification
{
    NSDictionary *dictionary = [notification userInfo];
    if ([notification.name isEqualToString:State] && [self.stateDictionary count]!=0) {
        NSString *stateId = [dictionary objectForKey:StateId];
        NSString *sender = [dictionary objectForKey:Sender];

        [self syncStateWithId:stateId andSender:sender];
    }
}

For each ARKView object, this method above attempts to find a state object that matches the name of the notification sent from another object, if it is found, it 'syncs' the state, giving the ARKView object the attributes it needs for that scene.

There is one performance issue that can be addressed quite easily, and that is when we're even bothering to respond to the notification.

This if statement can be completely eliminated:

if ([notification.name isEqualToString:State] && [self.stateDictionary count]!=0)

The left half of the && can be eliminated by registering only for a specific notification.  The right half of the && can be eliminated by registering/unregistering for the notification based on this property we're checking.

First, let's take care of registering/unregistering at appropriate times.

We only care about this notification if there's something in our stateDictionary, right?  So, let's unregister when it's empty, register when it's not.

- (void)setStateDictionary:(NSDictionary *)stateDictionary {
    _stateDictionary = stateDictionary;
    if ([stateDictionary count] > 0) {
        // register for notification
    } else {
        // unregister for notification
    }
}

Now our code is already better because our object is only getting a message from the notification center if our object is in a state where it would care about the notification.

Now, as for registering only for this specific notification.  That'd look something like this:

NSNotificationCenter *notifCenter = [NSNotificationCenter defaultCenter];
[notifCenter addObserver:self
                selector:@selector(receiveNotification:)
                    name:State
                  object:nil];

We can make this better still if we have a reference to the specific object that might post the notification (if there's only one) and only listen to notifications of that type.

The important bit here is that now we're only listening to notifications that have the notification name we care about and we're only listening when our object is in a state that we would care about the notification.  The if statement itself isn't a particularly expensive bit of code, however isEqualToString: isn't a particularly cheap method either, and there's no reason to run this (effectively twice, because NSNotificationCenter is already going to look at the notification names when determining who to send it to).

We can now refactor our method which responds to the notification into simply this:

- (void)receiveNotification:(NSNotification *)notification {
    NSDictionary *dictionary = notification.userInfo;
    NSString *stateId = dictionary[StateId];
    NSString *sender = dictionary[Sender];

    [self syncStateWithId:stateId andSender:sender];
}

And we'll get the same result.

So this code will run faster when it does receive the notification (because it skips the if check), and in general you'll be running faster overall because all the times that this method ran and the if check failed in the past, now the method will simply never be called and we'll never run through the if check.


Persistent login ("remember me" cookie) using PHP The code below is a attempt to create a persistent login cookie. I am an amateur and not professional so this is the first attempt to have secure remember me cookie. Now I want you guys to review two things:

I am trying to create a system like best way to implement remember me but I am not sure if I have done the same thing as explained there ( username row in table and cookie value isn't there but i have added).
Is my code a good way to secure remember me cookie ?

Any other suggestions or critiques are welcome.

    if (isset($_POST['rememberme'])) {

    $hash = sometext;
    $identifier = md5($username);
    $token = hash('sha512',$hash);
    setcookie('auth', $username. "," .$identifier. "," . $token, time()+31556926);
    $query=$db->prepare("SELECT username from tokens WHERE username=:username");
    $query->execute(array(':username'=>$username));
    if($query->rowCount() <=0 ){
    $result = $db->prepare("INSERT INTO tokens (username,identifier,token) VALUES (:username,:identifier,:token)");
    $result->execute(array(':username'=>$username,':identifier'=>$identifier,':token'=>$token));
     }
    }

    if(isset($_COOKIE["auth"])){
    $pieces = explode(",", $_COOKIE["username"]);
    $username = $pieces[0];
    $Uidentifier = $pieces[1];
    $Utoken = $pieces[2];

    $sql=$db->prepare("SELECT * FROM tokens WHERE identifier=:identifier");
    $sql->execute(array(':identifier'=>$Uidentifier));
    if($sql->rowCount()>0){
     $row = $sql->fetch(PDO::FETCH_ASSOC);
     $dbtoken = $row['token'];
     if($Utoken==$dbtoken){
     $newhash = 'sometext';
     $newtoken = hash('sha512',$newhash);
     $que=$db->prepare("UPDATE tokens SET token=:token WHERE identifier=:identifier");
     $que->execute(array(':token'=>$newtoken,':identifier'=>$Uidentifier));
     setcookie('auth', $username. "," .$identifier. "," . $token, time()+31556926);
     header("Location:home.php");
     }
     if($Utoken!=$dbtoken){
     echo "Unauthorized login attempt!";
     setcookie("auth","",time()-31556926);
     $stmt->$db=("DELETE FROM tokens WHERE identifier=:identifier");
     $stmt->execute(array(':identifier'=>$identifier));
     header("Location:login.php");
      }
     }
    }

Security

  I am not sure if I have done the same thing as explained there

  Is my code a good way to secure remember me cookie ?

No and no.

Non-Random Token: Login as anyone

The only data you store in your cookie is the username*. But usernames are generally not considered secret, and are relatively easy to obtain.

Because of this, an attacker can just calculate cookie=$username. "," .md5($username). "," . hash('sha512','sometext') themselves and log in as anyone.

You definitely always want at least partly random data in your cookie.

* and sometext. It's not entirely clear what this means, but because of the line $newhash = 'sometext';, I'm assuming that it is some hardcoded value, which is not good enough. Having (correctly generated) random data is essential here.

Non-Random Identifier: Invalidating Remember-Me Cookie of any user

Your identifier only consists of the (hashed) username.

An attacker can invalidate the remember-me cookie of any user by simply submitting a remember-me cookie like this: username,md5(username),invalid_token.

This is why the post suggests that the identifier should be random (otherwise, it wouldn't be needed, as the username could be used instead; the hashing itself doesn't add any security).

Plaintext Cookies: Login as anyone after database compromise

You also do not want to store the cookie as plaintext in the database, because it means that if an attacker gained access to your database, they can log in as anyone.

The hashing you do apply (md5 and sha512) do not solve this problem, as they are used to create the cookie, but not to store the cookie, or to compare the stored cookie to a user supplied cookie.

You should use bcrypt to store the cookie (the token part) in the database (just as you would when storing passwords).

Identifier and Token: Preventing timing attacks

The reason that the linked answer suggests using an identifier and a token is to prevent timing attacks which may result from a db lookup such as "SELECT * FROM tokens WHERE cookie=:user_supplied_cookie".

The problem here is that you actually use the identifier correctly, but then use ==, re-introducing the timing attack. If you use bcrypt as suggested, you will get a timing-safe string-compare for free.

httpOnly

Cookies such as this should be httpOnly to somewhat mitigate the risk of XSS attacks.

For more information on remember-me, this and this may help.

Misc

Your formatting is rather non-standard and partly inconsistent, making your code hard to read: Your indentation is inconsistent, and one space for indentation is definitely not enough. Your spacing is also inconsistent, and you could use more spaces (especially around comparisons such as !=, >, etc).
Don't shorten variable names, it makes your code hard to understand. What's a Utoken or a Uidentifier?
be consistent in the structure of your variable names. Some of your variables start with an upper-case character, some with a lower-case one, and all your variables are all lower-case afterwards, making them harder to read.
die after a header redirect, as a client does not have to actually follow the redirect.


Security of ProtectedObject Is ProtectedObject really secure? Otherwise, do you have any advice?

public class ProtectedBinary : IDisposable
{
    private byte[] pBArray;
    private int originPBArraySize;

    public ProtectedBinary(byte[] data)
    {
        originPBArraySize = data.Length;
        Pad(ref data);
        ProtectedMemory.Protect(data, MemoryProtectionScope.SameProcess);

        pBArray = new byte[data.Length];
        Array.ConstrainedCopy(data, 0, pBArray, 0, data.Length);
    }

    public void Get(out byte[] outArray)
    {
        ProtectedMemory.Unprotect(pBArray, MemoryProtectionScope.SameProcess);

        outArray = new byte[originPBArraySize];
        Array.Copy(pBArray, outArray, originPBArraySize);
        Unpad(ref outArray);

        ProtectedMemory.Protect(pBArray, MemoryProtectionScope.SameProcess);
    }

    public void Clear()
    {
        pBArray.Clear(); // set all bytes to 0x00
        originPBArraySize = 0;
    }

    public void Dispose()
    {
        Clear();
    }

    private void Pad(ref byte[] bytes)
    {
        Array.Resize(ref bytes, 16 * ((bytes.Length + 15) / 16));
    }

    private void Unpad(ref byte[] bytes)
    {
        Array.Resize(ref bytes, originPBArraySize);
    }
}

public class ProtectedObject<T> : IDisposable
{
    private ProtectedBinary pBin;

    private bool isDisposed;

    public ProtectedObject(T obj)
    {
        byte[] serializedObject = ULBinConvert.Serialize(obj);
        pBin = new ProtectedBinary(serializedObject);
    }

    public void Get(out T obj)
    {
        if (isDisposed)
            throw new ObjectDisposedException("Protected object is disposed");
        byte[] serializedObject;
        pBin.Get(out serializedObject);
        obj = ULBinConvert.Deserialize<T>(serializedObject);
        serializedObject.Clear();
    }

    public void Dispose()
    {
        if (!isDisposed)
        {
            pBin.Dispose();
            isDisposed = true;
        }
    }
}

Most of it looks fine to me, but why do you have void methods with out parameters? This makes no sense to me, and only serves to make usage more awkward. As such I would recommend:

public T Get()
{
    if (isDisposed)
        throw new ObjectDisposedException("Protected object is disposed");
    byte[] serializedObject = pBin.Get();
    T result = ULBinConvert.Deserialize<T>(serializedObject);
    serializedObject.Clear();
    return result;
}

Secondly, I don't like a lot of your variable names. You're not paying by the character, so there's no real benefit in shortening them at the cost of making it a slower read. pBin is significantly less readable than protectedBinary

You should also refrain from putting things like array into the variable name unless there is no better way to describe the variable. Most of the time it is unnecessary, as it is in pBArray or outArray.


Making adjustments based on biblatex style The styles which are provided by the biblatex package can be categorised into a few different families, e.g. numeric (numeric, numeric-comp and numeric-verb) or authoryear (authoryear, authoryear-comp, authoryear-ibid and authoryear-icomp). I'm currently working on a mechanism for beamer to apply some adjustments based on which of these style families is used. For example for numeric styles I would like the numbers to be shown in the bibliography or for authoryear styles I would like to prevent the icon from going into the margin.

My current version of the code tests for each style individually, e.g. three if-cases for the numeric styles and another four if-cases for the authoryear styles. This feels overly complicate, so I would like to hear what you think about this approach. Can it be improved?

\documentclass{beamer}

\usepackage[
%   style=numeric,
%   style=numeric-comp,
%   style=numeric-verb,
%   style=authoryear,
    style=authoryear-comp,
%   style=authoryear-ibid,
%   style=authoryear-icomp,
]{biblatex}

\addbibresource{biblatex-examples.bib}% included in full tex distributions or from http://mirrors.ctan.org/macros/latex/contrib/biblatex/doc/examples/biblatex-examples.bib

\makeatletter
\newcommand{\fixnumericstyles}{%
    \setbeamertemplate{bibliography item}{\insertbiblabel}
}

\newcommand{\fixauthoryearstyles}{%
    \newlength{\beamer@bibiconwidth}
    \settowidth\beamer@bibiconwidth{\usebeamertemplate*{bibliography item}}
    \setlength{\labelwidth}{-\beamer@bibiconwidth}
    \addtolength{\labelwidth}{2\labelsep}
    \addtolength{\bibhang}{\labelsep}
}

% Numeric styles %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
\def\beamer@namenumeric{numeric}
\ifx\blx@bbxfile\beamer@namenumeric
    \fixnumericstyles
\fi
%
\def\beamer@namenumericcomp{numeric-comp}
\ifx\blx@bbxfile\beamer@namenumericcomp
    \fixnumericstyles
\fi
%
\def\beamer@namenumericverb{numeric-verb}
\ifx\blx@bbxfile\beamer@namenumericverb
    \fixnumericstyles
\fi
%
% author year styles %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
\def\beamer@nameauthoryear{authoryear}
\ifx\blx@bbxfile\beamer@nameauthoryear
    \fixauthoryearstyles
\fi
\def\beamer@nameauthoryear{authoryear-comp}
\ifx\blx@bbxfile\beamer@nameauthoryear
    \fixauthoryearstyles
\fi
\def\beamer@nameauthoryear{authoryear-ibid}
\ifx\blx@bbxfile\beamer@nameauthoryear
    \fixauthoryearstyles
\fi
\def\beamer@nameauthoryear{authoryear-icomp}
\ifx\blx@bbxfile\beamer@nameauthoryear
    \fixauthoryearstyles
\fi
\makeatother
\begin{document}

\begin{frame}
\cite{sigfridsson}
\printbibliography
\end{frame}

\end{document}

(a variant of the above code incorporated into beamer can be found in this fork: https://github.com/samcarter/beamer/tree/PrivateCollectionBugFixes)

It seems I needed Aristotle to save my project -- with the help of Saving Aristotle: How can Biblatex redefinitions be made conditional on the style selected? the code can me massively simplified to the following example:
\documentclass{beamer}

\usepackage[
%   style=numeric,
%   style=numeric-comp,
%   style=numeric-verb,
%   style=authoryear,
%   style=authoryear-comp,
%   style=authoryear-ibid,
   style=authoryear-icomp,
]{biblatex}

\addbibresource{biblatex-examples.bib}% included in full tex distributions or from http://mirrors.ctan.org/macros/latex/contrib/biblatex/doc/examples/biblatex-examples.bib

\makeatletter
\newcommand{\fixnumericstyles}{%
    \setbeamertemplate{bibliography item}{\insertbiblabel}
}

\newcommand{\fixauthoryearstyles}{%
    \newlength{\beamer@bibiconwidth}
    \settowidth\beamer@bibiconwidth{\usebeamertemplate*{bibliography item}}
    \setlength{\labelwidth}{-\beamer@bibiconwidth}
    \addtolength{\labelwidth}{2\labelsep}
    \addtolength{\bibhang}{\labelsep}
}

% from https://tex.stackexchange.com/a/365563/36296
\def\beamer@blx@splitfile#1-#2\@null{#1}
\edef\beamer@blxbase{\expandafter\beamer@blx@splitfile\blx@cbxfile-x\@null}
%
% numeric styles %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
\edef\beamer@namenumeric{numeric}
\ifx\beamer@blxbase\beamer@namenumeric
    \fixnumericstyles
\fi
%
% author year styles %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
\edef\beamer@nameauthoryear{authoryear}
\ifx\beamer@blxbase\beamer@nameauthoryear
    \fixauthoryearstyles
\fi

\makeatother
\begin{document}

\begin{frame}
\cite{sigfridsson}
\printbibliography
\end{frame}

\end{document}

Update 2020
In the mean time, the biblatex developers were so nice to add some hooks which can be used for this purpose, see https://github.com/plk/biblatex/pull/973 for more details. This simplifies the solution dramatically and makes it much more versatile.
The following code was added to beamer to make the necessary adjustments:
% Adding patches to some biblatex styles
\csappto{blx@filehook@postload@numeric.bbx}{%
  \mode<presentation>{\setbeamertemplate{bibliography item}{\insertbiblabel}}}
\csappto{blx@filehook@postload@alphabetic.bbx}{%
  \mode<presentation>{\setbeamertemplate{bibliography item}{\insertbiblabel}}}
\csappto{blx@filehook@postload@authoryear.bbx}{%
  \mode<presentation>{%
      \newlength{\beamer@bibiconwidth}
      \settowidth\beamer@bibiconwidth{\usebeamertemplate*{bibliography item}}
      \setlength{\labelwidth}{-\beamer@bibiconwidth}
      \addtolength{\labelwidth}{2\labelsep}
      \addtolength{\bibhang}{\labelsep}
  }}
\csappto{blx@filehook@postload@authortitle.bbx}{%
  \mode<presentation>{%
      \newlength{\beamer@bibiconwidth}
      \settowidth\beamer@bibiconwidth{\usebeamertemplate*{bibliography item}}
      \setlength{\labelwidth}{-\beamer@bibiconwidth}
      \addtolength{\labelwidth}{2\labelsep}
      \addtolength{\bibhang}{\labelsep}
  }}


Implementing POSIX file mode library I implemented a virtual file system library with POSIX file mode representation.
Any suggestions regarding optimization are welcome.

"""POSIX file system library."""

from collections import namedtuple

__all__ = ['PosixPermission', 'PosixMode']

MIN_PERM = 0b000
MAX_PERM = 0b111
DEF_PERM = 0b110    # rw-
MIN_MODE = 0o000
MAX_MODE = 0o777
DEF_MODE = 0o640

class PosixPermission(namedtuple('PosixPermission', 'read write execute')):
    """POSIX file permissions."""

    @classmethod
    def from_int(cls, perm=DEF_PERM):
        """Creates the file permissions from the respective integer value."""
        if MIN_PERM <= perm <= MAX_PERM:
            return cls(bool(perm & 4), bool(perm & 2), bool(perm & 1))

        raise ValueError('Permission value must be between {} and {}.'.format(
            MIN_PERM, MAX_PERM))

    def __int__(self):
        """Returns the integer value of the file permission."""
        return (self.read and 4) + (self.write and 2) + (self.execute and 1)

    def __str__(self):
        """Returns binary representation of the file permission."""
        return ''.join(map(
            lambda perm: perm or '-',
            (self.read and 'r', self.write and 'w', self.execute and 'x')))

class PosixMode(namedtuple('PosixMode', 'user group other')):
    """POSIX file mode."""

    @classmethod
    def from_int(cls, mode=DEF_MODE):
        """Creates the file mode from the respective integer value."""
        if MIN_MODE <= mode <= MAX_MODE:
            return cls(
                PosixPermission.from_int(mode >> 6 & 7),
                PosixPermission.from_int(mode >> 3 & 7),
                PosixPermission.from_int(mode & 7))

        raise ValueError('File mode must be between {} and {}.'.format(
            oct(MIN_MODE), oct(MAX_MODE)))

    def __int__(self):
        """Returns the integer value of the file mode."""
        return (int(self.user) << 6) + (int(self.group) << 3) + int(self.other)

    def __str__(self):
        """Returns the octal representation of the file mode."""
        return ''.join((str(self.user), str(self.group), str(self.other)))

I'm confused why you're using a named tuple for PosixPermission. To me an Enum would be a better choice:

class PosixPermission(Flag):
    READ = 4
    WRITE = 2
    EXECUTE = 1
    NONE = 0

    @property
    def perms(self):
        return ''.join(
            name if self & val else '-'
            for name, val in zip('rwx', PosixPermission)
        )

>>> PosixPermission(0)
<PosixPermission.NONE: 0>
>>> PosixPermission(7)
<PosixPermission.READ|WRITE|EXECUTE: 7>
>>> PosixPermission(5)
<PosixPermission.READ|EXECUTE: 5>
>>> PosixPermission(5).perms
'r-x'
>>> PosixPermission(5) & PosixPermission.READ
<PosixPermission.READ: 4>
>>> PosixPermission(5) & PosixPermission.WRITE
<PosixPermission.NONE: 0>
>>> PosixPermission(8)
Traceback (most recent call last):
ValueError: 8 is not a valid PosixPermission

Honestly PosixMode looks pretty good, I think it's reasonable to assume it to be a namedtuple, rather than an object with __slots__. I however wouldn't use __int__ and __str__, and would instead use properties for these.


Text class in Python Here is a class named Text, which takes a string as its argument of its constructor. It has property-methods to get the words in the string and also the frequency of alphabets occuring in it.

The words property result in all words in the string. It works as follows, after using the .split method, there is a possibility that a "word" is of the form ,word or word. or ,word. etc. So to encounter this we check each end using .isalpha.

We also have find_words method, which has words as its inputs. This method will summarize the frequencies of a set of words.

How can I write this better to:

The level of a library/package (I don't really understand the techniques for this)
More efficient and readable (using re module is last option)
More functional, but compact

#Author: Arief. A

example = "This is an example, an Anonymous text.\
 This text has 59 letters or numbers.";

class Text:

    def __init__(self, input_text):
        self.text = input_text;

    def show(self):
        print(self.text);

    def as_list(self):
        return list(self.text);

    def get_length(self):
        return len(self.text);

    def count_char(self, char):
        return self.text.count(char);

    @property
    def letter_freqs(self):
        letters="abcdefghijklmnopqrstuvwxyz";
        res={i: self.count_char(i) \
             for i in letters};
        caps={i.capitalize(): self.count_char(i.capitalize()) \
             for i in letters};
        res.update(caps);
        return res

    @property
    def words(self):
        raw=self.text.split();
        for i in range(len(raw)):
            if not raw[i][0].isalpha():
                raw[i]=raw[i][1:];
            if not raw[i][-1].isalpha():
                raw[i]=raw[i][0:-1];
        [raw.remove('') for i in range(raw.count(''))];
        return raw

    def swap_first(word):
        swaped=word[0].swapcase();
        res=swaped+word[1:];
        return res

    def find_words(self, *args):
        return {i: self.words.count(i)\
                      +self.words.count(Text.swap_first(i))\
                      for i in args};

Example

text_obj=Text(example);
text_obj.show();
print(text_obj.as_list());
print(text_obj.get_length());
print(text_obj.count_char("a"));
print(text_obj.words)
print(text_obj.find_words("Anonymous", "Text","This"));

Output

This is an example, an Anonymous text. This text has 59 letters or numbers.
['T', 'h', 'i', 's', ' ', 'i', 's', ' ', 'a', 'n', ' ', 'e', 'x', 'a', 'm', 'p', 'l', 'e', ',', ' ', 'a', 'n', ' ', 'A', 'n', 'o', 'n', 'y', 'm', 'o', 'u', 's', ' ', 't', 'e', 'x', 't', '.', ' ', 'T', 'h', 'i', 's', ' ', 't', 'e', 'x', 't', ' ', 'h', 'a', 's', ' ', '5', '9', ' ', 'l', 'e', 't', 't', 'e', 'r', 's', ' ', 'o', 'r', ' ', 'n', 'u', 'm', 'b', 'e', 'r', 's', '.']
75
4
['This', 'is', 'an', 'example', 'an', 'Anonymous', 'text', 'This', 'text', 'has', 'letters', 'or', 'numbers']
{'This': 2, 'Text': 2, 'Anonymous': 1}

Instead of letters="abcdefghijklmnopqrstuvwxyz"; use a built-in constant:

letters = string.letters;

The code only inspects first and last character of the word. It means that aaa.bbb will be treated as a single word. Is this an intent?
split(sep) allows a string of separators. Calling it with

raw = self.text.split(string.punctuation + string.whitespace)

eliminates the need to inspect individual words. If you still want to inspect them, a loop for i in range(len(raw)): should be written more idiomatically as

for word in raw:

(you don't really need i anywhere in the loop).


Find minimum singular value in a sorted array A sorted array is passed to my method and i have to find the minimum singular value. If no such value is found the method returns 0.
I have created two versions of this method. In the first version, I iterate trough the array twice which works fine but there has to be a way to find the value with one iteration.
In the second version, I iterate through it once but i need it fixed so, that it works if the last index of the array is my minimum singular value e.g {1, 1, 2, 2, 3}.

//First version with two iterations
public int findMinimumSingularValue(int[] array) {

    for (int i = 0; i < array.length; i++) {
        int countOccurence = 0;
        for (int j = 0; j < array.length; j++) {
            if (array[i] == array[j]) {
                countOccurence ++;
            }
        }
        if(countOccurence == 1) {
            return array[i];
        }
    }
    return 0;
}

I've tried to implement it with only one iteration, but couldn't get it to work. Only the first version is up for review, but I'll leave my attempt at improvement here to illustrate what I have in mind:

//Second version with one iteration which needs to be fixed
public int findMinimumSingularValue(int[] array) {
    if(array.length == 0) {
        return 0;
    }
    int isNotUnique = array[array.length - 1];
    for (int i = 0; i < array.length; i++) {
        if (array[i] != array[i + 1] && array[i] != isNotUnique) {
            return array[i];
        }
        isNotUnique = array[i];
    }
    return 0;
}

for (int i = 0; i < array.length; i++) {
        int countOccurence = 0;
        for (int j = 0; j < array.length; j++) {
            if (array[i] == array[j]) {
                countOccurence ++;
            }
        }
        if(countOccurence == 1) {
            return array[i];
        }
    }
    return 0;

When I look at this, the problem that I see is that you are scanning the entire array to look for duplicates.  The array is sorted!  Consider

    int i = 0;
    while (i + 1 < array.length) {
        // array[i] has a duplicate iff it is the same as array[i + 1]
        if (array[i] != array[i + 1]) {
            return array[i];
        }

        // skip to the next possible smallest value not a duplicate
        while (i + 1 < array.length && array[i] == array[++i]) ;
    }

    // figure out if the last element has no duplicate
    return (array.length == 0 || array[i] == array[i - 1]) ? 0 : array[i];

While it may look like it has nested loops, both iterate over the same variable.  So the number of comparisons is linear relative to the size of the array.

This works because the array is sorted.  All duplicates are adjacent to each other.

The ++i increments i and then returns the now incremented value.  i++ stores the value of i, then increments it, and finally returns it.  So i++ + 1 is the same thing as ++i.  Or vice versa.

class Incrementable {

    private int i = 0;

    // ++i
    public int preincrement() {
        i = i + 1;
        return i;
    }

    // i++
    public int postincrement() {
        int original = i;
        i = i + 1;
        return original;
    }

}

The two forms ++i and i++ are the equivalents of the preincrement and postincrement in this class.

So let's go through

        while (i + 1 < array.length && array[i] == array[++i]) ;

The form

while (isTrue()) ;

just keeps iterating until isTrue returns false.  It does nothing in the body of the loop, thus the empty ;.  We could also write it

while (isTrue()) {

}

But that just seems longer.

The i + 1 < array.length just keeps us from falling off the back of the array.  I.e. we never want to do array[array.length], as that's an error (attempt to access outside the array).

So the meat of the statement is the array[i] == array[++i].  On any particular iteration, this is just the same as array[i] == array[i + 1].  So long as that is true, we're still processing the same duplicate.  But replacing i + 1 with ++i has the same effect on an individual iteration and moves along.  So we have something like an array of

  1, 1, 2, 2, 2, 2, 3

i = 0, array[i] = 1, array[++i] = 1, i = 1; equal so keep going
i = 1, array[i] = 1, array[++i] = 2, i = 2; not equal so stop and do the outer while
i = 2, array[i] = 2, array[++i] = 2, i = 3; equal so keep going
i = 3, array[i] = 2, array[++i] = 2, i = 4; equal so keep going
i = 4, array[i] = 2, array[++i] = 2, i = 5; equal so keep going
i = 5, array[i] = 2, array[++i] = 3, i = 6; not equal so stop and do the outer while

While i is 6, we'll drop out of the inner loop and the outer loop, because 6 + 1 is not less than array.length, which is 7.


Showing jQuery UI tooltip on focus of text box Firstly, here's a link to the fiddle.

Basically my situation is that there is a requirement to show tooltips on textboxes on some forms being created for a website to allow employees to update their personal information (among other things).

The customer likes the idea of the tooltip being displayed to the right of the textbox when the user clicks into it rather than jQuery UI's default of when hovering over it. To that end, I created the following code that shows and hides the tooltip when the user clicks into/out of the textbox:

$(function () {
    $('.tooltip').each(function () {
        var $this,
            id,
            t;

        $this = $(this);
        id = this.id;
        t = $('<span />', {
            title: $this.attr('title')
        }).appendTo($this.parent()).tooltip({
            position: {
                of: '#' + id,
                my: "left+190 center",
                at: "left center",
                collision: "fit"
            }
        });
        // remove the title from the real element.
        $this.attr('title', '');
        $('#' + id).focusin(function () {
            t.tooltip('open');
        }).focusout(function () {
            t.tooltip('close');
        });
    });
});

My question is simple, is there a better way of doing this?

Note that I can rely on the inputs always having a unique Id as this is an ASP.Net application.

The demo is seen here

$('.tooltip').each(function () {

    //title can be fetched directly from the element. I know no bugs for to
    //require the use of `attr()`. This removes several function calls.

    //No need to get the element by id since the current elements you are iterating
    //over each are the same DOM elements you are trying to attach events to. The
    //`this` in here is where you attach events.

    //You are appending the content to the parent, landing the span next to input.
    //Instead of doing that, you can use `insertAfter()` which inserts the object in
    //context after  the specified element, which is the current object in the
    //`each()`. It also accepts a DOM element so no need to wrap it in jQuery.

    var t = $('<span />', {
        title: this.title
    }).insertAfter(this).tooltip({
        position: {
            of: '#' + this.id,
            my: "left+190 center",
            at: "left center",
            collision: "fit"
        }
    });

    this.title = '';

    //Now the only need to wrap the current object this is here, when we attach events.

    //Since the event methods are merely shorthands of `on(eventName,fn)`, we use
    //`on()` instead to lessen function calls. Also, it allows a map of events, which
    //results in calling `on()` only once.

    $(this).on({
        focusin: function () {
            t.tooltip('open');
        },
        focusout: function () {
            t.tooltip('close');
        }
    });
});


Determine if points are within a rotated rectangle Using only Python 2.7 and the standard library (no imports):
Determine if a point is inside the rotated rectangle.
Test point 1: 670831  4867989
Test point 2: 675097  4869543

Rectangle:

Vertex 1:     670273   4879507
Vertex 2:     677241   4859302
Vertex 3:     670388   4856938
Vertex 4:     663420   4877144

Can the following code be improved?
vertices = [(670273, 4879507), (677241, 4859302), (670388, 4856938), (663420, 4877144)]

def dot_pdt(point1, point2, pointT):
    return ((point2[0] - point1[0]) * (pointT[0] - point1[0])) + ((point2[1] - point1[1]) * (pointT[1] - point1[1]))

def check_point(pointT):
    for i in range(len(vertices)):
        k = dot_pdt(vertices[i], vertices[(i + 1) % len(vertices)], pointT)
        if k <= 0:
            return False

    return True

point1 = (676455.270, 4861545.437)

print(check_point(point1))

Code Review

Main guard

When running code outside a function / class, it is a good practice to put the code inside the main guard. See here for more explanation.

Docstrings

It is a good practice to provide docstrings for functions. See here for more details. In Python 3.5+, adding type hints to function parameters and return values is also a good habit. (As a side remark, Python 2 has officially reached the end of life. It is recommended to move to Python 3.)

Variable & function names should be clear, descriptive, and unambiguous

For example, check_point could be named as is_inside_rectangle.

Function definition should include all necessary parameters

vertices should be a parameter of the function check_point.

Other minor improvements

By exploiting the fact that vertices[-1] is the same as vertices[len(vertices)-1], the explicit modulo operation vertices[(i + 1) % len(vertices)] could be avoided.
The for-loop logic can be simplified using an all statement.
Here is an improved version of the code:
def dot_prod_with_shared_start(start, end1, end2):
    """
    Compute the dot product of the vectors pointing from start to end1 and end2

    Parameters
        start: starting point of both vectors
        end1: end point of first vector
        end2: end point of second vector

    Returns
        dot product of (end1 - start) and (end2 - start)
    """
    return (end1[0] - start[0]) * (end2[0] - start[0]) + (end1[1] - start[1]) * (end2[1] - start[1])

def is_inside_rectangle(vertices, point):
    """
    Given the vertices of a rectangle, determine whether a point is inside it.

    Parameters
        vertices: a list of tuples representing rectangle vertices in clockwise or
                  counter-clockwise order
        point: a tuple representing the point to check

    Returns
        True if the point is inside the rectangle. False otherwise.
    """
    return all(dot_prod_with_shared_start(vertices[i - 1], v, point) > 0 for i, v in enumerate(vertices))

if __name__ == "__main__":
   rect_vertices = [(670273, 4879507), (677241, 4859302), (670388, 4856938), (663420, 4877144)]
   test_point = (676455.270, 4861545.437)

   print(is_inside_rectangle(rect_vertices, test_point))

Another Solution
An alternative solution is to represent the vertices and the point as complex values, and apply complex number arithmetics to solve the problem.
def is_directed_angle_acute(start, end1, end2):
    """
    Check whether the directed angle from (end1 - start) to (end2 - start) is an acute angle,
    i.e. within (0, pi / 4).

    Parameters
        start: a complex value representing the starting point of both vectors
        end1: a complex value representing the end point of first vector
        end2: a complex value representing the end point of second vector

    Returns
        True if the directed angle from (end1 - start) to (end2 - start) is within (0, pi / 4).
        False otherwise.
    """
    q = (end2 - start) / (end1 - start)
    return q.real > 0 and q.imag > 0

def is_inside_rectangle(vertices, point):
    """
    Given the vertices of a rectangle, determine whether a point is inside it.

    Parameters
        vertices: a list of complex values representing rectangle vertices in clockwise order
        point: a complex value representing the point to check

    Returns
        True if the point is inside the rectangle. False otherwise.
    """
    v0, v1, v2, v3 = vertices
    return is_directed_angle_acute(v0, point, v1) and is_directed_angle_acute(v2, point, v3)

if __name__ == "__main__":
   rect_vertices = [670273 + 4879507j, 677241 + 4859302j, 670388 + 4856938j, 663420 + 4877144j]
   test_point = 676455.270 + 4861545.437j

   print(is_inside_rectangle(rect_vertices, test_point))


A simple web scraper I've written a simple web scraper in Python. As it is my first venture in Python, I'd like to ask for a code review from more experienced coders. All input is welcome.

A little bit of background:

I'm a huge fan of video games. In Japan, there is a company called Media Create which tracks all sales of games and consoles and releases them weekly (in Japanese language). Another site (www.gematsu.com) translates those charts and releases them weekly on their website. My script fetches the data and saves it to a text file.

The script should run as-is, but it is dependent on a text file of the previous week existing in script-location/data.

The name of the script needs to be something like media-create-sales-11-26-18-12-2-18.txt (replacing the start and end dates with any dates other than the dates of the most recently released data).

I am not affiliated with Media Create or Gematsu and this script serves no commercial purpose.

import requests
from bs4 import BeautifulSoup
import datetime
import glob
import os

def getLatestFilename():

    list_of_files = glob.glob('data/*')
    latest_file = max(list_of_files, key=os.path.getctime)

    return latest_file

def parseUrl(dateString):

    dateString = dateString.split('-')

    lastdateString = str("-".join(dateString[6:9])).split('.')[0]

    lastdate_object = datetime.datetime.strptime(lastdateString, "%m-%d-%y")

    current_startdatetime_object = lastdate_object + datetime.timedelta(days=1)
    current_enddatetime_object = lastdate_object + datetime.timedelta(days=7)

    cur_startDateString = current_startdatetime_object.strftime("%m-%d-%y").lstrip("0").replace("0", "")
    cur_endDateString = current_enddatetime_object.strftime("%m-%d-%y").lstrip("0").replace("0", "")

    return (cur_startDateString + '-' + cur_endDateString)

def getUrl(filename):

    curIntervalString = parseUrl(filename)

    now = datetime.datetime.now()

    url = list()
    url.append("https://gematsu.com/")
    url.append(str(now.year))
    url.append('/')
    url.append(str(now.month))
    url.append('/')
    url.append('media-create-sales-')
    url.append(curIntervalString)

    return url

def fetchContentForUrl(url):

    page = requests.get(url)
    soup = BeautifulSoup(page.text, 'html.parser')

    outputfile = open("data/media" + url.split('media')[1] + ".txt", "w")

    outputfile.write(url + '\n' + '\n')

    content = soup.find(class_='entry')

    gameCharts = content.findNext('ol')
    gameEntries = gameCharts.find_all('li')

    for idx,entry in enumerate(gameEntries,start=1):
        outputfile.write(str(idx) + "." + entry.get_text() + "\n")

    print ('\n')

    consoleCharts = gameCharts.find_next_sibling("ol")
    consoleEntries = consoleCharts.find_all('li')

    for entry in consoleEntries:
        outputfile.write(entry.get_text() + "\n")

def main():
    latestFilename = getLatestFilename()
    print (latestFilename)
    url = ''.join(getUrl(latestFilename))
    print (url)
    fetchContentForUrl(url)

if __name__ == '__main__':
    main()

This is a very good attempt for the first Python venture!

Here is what I would consider improving:

stylistic / PEP8

naming. In Python, it is recommended to use lower_case_with_underscores notation. Your function and variable names are inconsistent and there is a mix of styles - e.g. cur_startDateString follows both camelCase and lower_case_with_underscores
the user of blank lines. In Python, it is recommended to have 2 blank lines between top-level blocks of the code and have an extra new-line at the end of the script
the use of whitespaces. Watch for whitespaces after commas in expressions, e.g.:

for idx,entry in enumerate(gameEntries,start=1):
   ^                              ^

import grouping and order. See Imports section of the PEP8
parenthesis around the return value are redundant
consider adding docstrings explaining what your functions are about. For instance, getLatestFilename would benefit from a note about how does it define which file is the latest. And, parseUrl is not really easy to understand just by reading the code
if you are using Python 3.5+, consider adding Type Hints

Design and modularity

it is a good thing that you tried to logically separate scraping from parsing
I would also separate parsing and dumping the results into a CSV file

HTML Parsing

if performance is a concern, consider switching from html.parser to lxml - see "Installing a Parser" section of the documentation
you could also improve performance of the parsing stage by using a SoupStrainer which allows you to parse the desired part of the HTML tree in the first place:

from bs4 import BeautifulSoup, SoupStrainer

parse_only = SoupStrainer(class_='entry')
soup = BeautifulSoup(page.text, 'lxml', parse_only=parse_only)

.findNext() is deprecated in favour of .find_next()

Other

getUrl() method feels a little bit overcomplicated and you can definitely improve its readability and simplify it at the same time. Plus, I would make it return the URL as a string and not as a list to avoid any extra external code needed to make it a string. If you are using Python 3.6+, f-strings would be handy:

def get_url(filename: str) -> str:
    """Constructs a complete URL based on the filename and current date."""
    current_interval = parse_url(filename)

    now = datetime.datetime.now()

    return f"https://gematsu.com/{now.year}/{now.month}/media-create-sales-{current_interval}"

Having the complete URL string with placeholders helps to see what the end result might look like.

Note that most of the PEP8 violations mentioned above could be caught by running a linter (like flake8, or pylint) against your code.


Static quiz module with predefined answers I was building a quiz module for one website. It is a simple true/false statements where you first choose the answer, the correct answer is displayed, and then you can move forward to the next question.

For better understanding, please find working code snippet here.

I need mentor guidance in order to make my existing code shorter and optimized.

Here is the jQuery code for this quiz module:

var flag = $('.quiz ul'),
    currentPosition = 0,
    currentQuestion = 0,
    slideWidth = 200,
    option = $('.flag'),
    tryCTA = $('.CTAtry'),
    nextQuestion = $('.next'),
    answer = $('span.answer')
    questionWrapper = $('.quiz ul'),
    totalQuestions = $('.quiz ul li').length,
    answers = ["True","False","False"];

//Set UL width
questionWrapper.css('width',totalQuestions*slideWidth)

  option.on('click',function(e) {
      e.preventDefault();
      var nextCTA = $(this).parent().next(nextQuestion).length,
          optionWrapper = $(this).parent();
      if(nextCTA) {
          optionWrapper.next(nextQuestion).show();
      } else {
          tryCTA.show();
      }
      optionWrapper.parent().find(answer).html(answers[currentQuestion]).show();
      optionWrapper.hide();
  });

  nextQuestion.on('click',function(e){
      e.preventDefault();
      currentPosition = currentPosition+1;
      $(this).parent().parent().animate({
          'marginLeft' : slideWidth*(-currentPosition)
      });
      currentQuestion++;
  });

  tryCTA.on('click',function(e) {
      e.preventDefault();
      resetQuiz(e);
      $(this).hide();
  })

  function resetQuiz(e) {
      $('.quiz ul').animate({
           opacity: 0
      },0)
      .animate({
           opacity: 1
       },500)

      $('span.answer').html(" ");
      $('.quiz ul').css('margin-left','0');
      $('.options').show();
      $('.next').hide();
      currentPosition = 0;
      currentQuestion = 0;
  }

HTML:

Writing Object Oriented JavaScript is not a FUN.

            True
            or
            False

        Next

Let's give it a try! No harm in learning anything NEW.

            True
            or
            False

        Next

Some code running !

            True
            or
            False

Try Again!

Overall this is already pretty well organized and compact.  There isn't a lot of duplication or obvious abstractions that can be made.  It would be helpful to see the HTML to see if there are any changes that would make the Javascript easier to write.
A couple of suggestions:

Wrap the entire code block in an IIFE to avoid cluttering global namespace
 (function ($, undefined) {
     // Your code here ...
 } (jQuery));

Use .width() instead of .css('width', ...)

Some of your variables are only being referenced once and can be inlined:
 $('.flag').on('click', ...); // etc.


Build object from database without a discriminator column? I currently have a basic application that has a database backing it. I originally connected to the database with Entity Framework, but my tutor has requested from me to recreate the functionality with ADO.NET SQL. This has gone well so far using the SqlDataReader, however I have come across a problem.

My database stores a list of animals, the animals are Dog, Cat and Mouse, they all inherit from the Animal class. With entity framework when I wanted to get an animal from the database, it used the discriminator column automatically and I didn't have to worry about any logic to detect what type of animal the animal was. However so far the only way I can think of doing this with SQL is to manually check the discriminator column.

This works well for the moment but if I start adding say a hundred animals, it will become an impossible task to keep it working efficiently. My question is, is there another way to do this which is more elegant and will scale appropriately? The code for how I currently do the task is below:

if (reader.HasRows)
            {
                while (reader.Read())
                {
                    if (reader.GetString(3) == "Dog")
                    {
                        list.Add(
                            new Dog()
                            {
                                AnimalID = (int)reader.GetInt32(reader.GetOrdinal("AnimalID")),
                                Name = (string)reader.GetString(reader.GetOrdinal("Name")),
                                Age = (int)reader.GetInt32(reader.GetOrdinal("Age")),
                            });
                    }

                    if (reader.GetString(3) == "Cat")
                    {
                        list.Add(
                            new Cat()
                            {
                                AnimalID = (int)reader.GetInt32(reader.GetOrdinal("AnimalID")),
                                Name = (string)reader.GetString(reader.GetOrdinal("Name")),
                                Age = (int)reader.GetInt32(reader.GetOrdinal("Age")),
                            });
                    }

                    if (reader.GetString(3) == "Mouse")
                    {
                        list.Add(
                            new Mouse()
                            {
                                AnimalID = (int)reader.GetInt32(reader.GetOrdinal("AnimalID")),
                                Name = (string)reader.GetString(reader.GetOrdinal("Name")),
                                Age = (int)reader.GetInt32(reader.GetOrdinal("Age")),
                            });
                    }
                }
            }
            else
            {
                //no rows
            }

Here is your code refactored - since all animals have id, name and age, you can remove duplication (reading and setting these properties for each type of animal):

if (reader.HasRows)
{
    while (reader.Read())
    {
         string animalType = reader.GetString(3);

         Animal animal = CreateAnimal(animalType);
         animal.AnimalID = (int)reader["AnimalID"];
         animal.Name = (string)reader["Name"];
         animal.Age = (int)reader["Age"];
         list.Add(animal);
    }
}

Animal creation (if you are using type name as discriminator you can even use Activator.CreateInstance instead of this switch to create an instance of class):

private Animal CreateAnimal(string animalType)
{
     switch(animalType)
     {
        case "Dog": return new Dog();
        case "Cat": return new Cat();
        case "Mouse": return new Mouse();
        default:
             throw new ArgumentException();
     }
}


Brute force solution to Project Euler Problem #52 I was solving Project Euler Problem #52:

It can be seen that the number, 125874, and its double, 251748, contain exactly the same digits, but in a different order.
Find the smallest positive integer, x, such that 2x, 3x, 4x, 5x, and 6x, contain the same digits.

in C++, and this is the code I used:
#include <algorithm>
#include <iostream>
#include <string>

bool checkCondition(long long int number);

int main()
{
    long long int num = 1;
    while (!checkCondition(num))
    {
        ++num;
    }

    std::cout << num << "\n";
}

bool checkCondition(long long int number)
{
    std::string numString = std::to_string(number);
    std::string numx2 = std::to_string(number * 2);
    std::string numx3 = std::to_string(number * 3);
    std::string numx4 = std::to_string(number * 4);
    std::string numx5 = std::to_string(number * 5);
    std::string numx6 = std::to_string(number * 6);

    return std::is_permutation(numString.begin(), numString.end(), numx2.begin()) &&
        std::is_permutation(numString.begin(), numString.end(), numx3.begin()) &&
        std::is_permutation(numString.begin(), numString.end(), numx4.begin()) &&
        std::is_permutation(numString.begin(), numString.end(), numx5.begin()) &&
        std::is_permutation(numString.begin(), numString.end(), numx6.begin());
}

As evident, this function uses the standard library function std::is_permutation and literally iterates over every single integer and converts them to strings to find the answer, which is obviously inefficient. How can I improve upon the efficiency of this program?

Before I begin: I solved this particular Project Euler problem in 10 seconds without even writing a program, because I had seen a Numberphile video on this concept and I had memorized the number. It's really just a mathematical problem!

Whenever you must do brute force, you have two primary goals:

Reduce the number of trials by eliminating some definitely incorrect cases. In this example, if N and N * 6 doesn't have the same number of digits, they cannot be the permutations of each other. In our case this won't be much useful, but if you did some maths, you would notice that you would only have to check numbers between 100000..~170000, 200000..~270000, because numbers outside those ranges don't satisfy the condition stated above. If we were asked to find a much larger solution, using this approach would have eliminated about 30% of the options right away.
Reduce the amount of effort you must give at each trial. In this case, if 2 * N isn't a permutation of N, you shouldn't check for 3 * N because you know that you aren't getting a solution.

Finally, if I were you, I wouldn't use string operations. You can convert each number into a sorted list of digits in it, and then check for list equality.

Combining all of the above, you could write something like this:

A = sorted list of digits in (current trial)
for i = 2 to 6 inclusive:
    B = sorted list of digits in (current trial times i)
    if A and B are different, fail
succeed! -- can only reach here if all former multiples are permutations


Django Calendar Model I am building a doctor appointment app with Django and Django Rest Framework. The app will have the functionality for secretaries to add appointments to a Doctor's calendar, and each Doctor may have more than one clinic (and multiple secretaries).
I built this Calendar Model to store the availability information of a doctor, which needs to basically inform:

on a given day, at what time the doctor starts and finishes work
on a given day, at what time the doctor starts and finishes their break time (if they do).

Is this the optimal way to do it?
class Calendar(models.Model):
    doctor = models.ForeignKey(Doctor, on_delete=models.CASCADE, related_name="appointments")
    clinic = models.ForeignKey(Clinic, on_delete=models.CASCADE, related_name="appointments")
    monday_start =  models.TimeField()
    monday_end =  models.TimeField()
    monday_break_start =  models.TimeField()
    monday_break_end =  models.TimeField()
    # continues for every single day of the week

This is relatively simple solution you have here and it could in theory work, but in my experience in a real word you need something more flexible. Also you dont think about redundancy of the data.
What if a doctor for example works only one day in a week. Most of the columns would then be empty. Or if a doctor can work only in the morning? Then there would be no break_start or end in any of the calendar fields. What if, for example, the doctor works every second week? Then you will not have an easy way how to store this information. Also there would be a lot of redundant information. If you will have each doctor working from 8:00 AM to 4:00 PM, you will have this data stored there redundantly. Much less space is needed, if you can store such information as a relation to some kind of duration record.
If I were you, I would create more generic design where you can store up to 365 records (for each day in the year), for each calendar. Then each day would have a relation to a list of intervals, that could later be of required activity. That way you would end up with more flexible design.
The draft could look something like this:
class Calendar(models.Model):
    doctor = models.ForeignKey(Doctor, on_delete=models.CASCADE)
    clinic = models.ForeignKey(Clinic, on_delete=models.CASCADE)

class CalendarDay(models.Model):
    date = model.DateField()
    calendar = models.ForeignKey(Calendar, on_delete=models.CASCADE)

class TimeInterval(models.Model):
    activity = models.Integer(choices = [(1, 'Working'), (2,'Lunch Break'), (3, 'Afternoon Break') ...])
    start = models.TimeField()
    end = models.TimeField()
    calendar_day = models.ForeignKey(CalendarDay, on_delete=models.CASCADE)

In order to ensure data integrity, then you could add an unique constraint on the combination of date and calendar columns in the CalendarDay table. That way each calendar can have only one record for particular date in the CalendarDay table.


Given an array of numbers, return array of products of all other numbers I was recently asked the following interview question over the phone:

  Given an array of integers, produce an array whose values are the
  product of every other integer excluding the current index.

  Example:

  [4, 3, 2, 8] -> [3*2*8, 4*2*8, 4*3*8, 4*3*2] -> [48, 64, 96, 24]

I came up with below code:

  public static BigInteger[] calcArray(int[] input) throws Exception {
    if (input == null) {
      throw new IllegalArgumentException("input is null");
    }

    BigInteger result[] = new BigInteger[input.length];
    for (int i = 0; i < input.length; i++) {
      result[i] = calculateProduct(input, i);
    }
    return result;
  }

  private static BigInteger calculateProduct(int[] input, int exceptIndex) {
    BigInteger result = BigInteger.ONE;
    for (int i = 0; i < input.length; i++) {
      if (i == exceptIndex) {
        continue;
      }
      result = result.multiply(BigInteger.valueOf(input[i]));
    }
    return result;
  }

Complexity:

Time Complexity: O(n)
Space Complexity: O(n)

Is there any better or efficient way to do this apart from what I have so that complexity can be reduced?

The Time Complexity is indeed: \$\mathcal{O}(n^2)\$.

You can optimize it to \$\mathcal{O}(n)\$ by calculating the total product of all elements in the source array, and then looping over the array and dividing the product by each element to get the product excluding that particular element.

Code:

public static BigInteger[] calcArray(int[] input) throws Exception {
      if (input == null) {
        throw new IllegalArgumentException("input is null");
      }

      BigInteger product = calculateProduct(input);
      BigInteger result[] = new BigInteger[input.length];
      for (int i = 0; i < input.length; i++) {
        result[i] = product.divide(BigInteger.valueOf(input[i]));
      }
      return result;
}

private static BigInteger calculateProduct(int[] input) {
      BigInteger result = BigInteger.ONE;
      for (int i = 0; i < input.length; i++) {
          result = result.multiply(BigInteger.valueOf(input[i]));
      }
      return result;
}

EDIT: The above code assumes that none of the numbers is 0 in the input array.


Getting a substructure in a JSON-file I'm currently playing with D3.js.

While doing so I use this JSON-file as a data source: http://codepen.io/chriscoyier/pen/taqCe

Example of the structure:

{
  "fathers" : [
    {
      "id" : 0,
      "married" : false,
      "name" : "Robert Wilson",
      "sons" : null,
      "daughters" : [
        {
          "age" : 21,
          "name" : "Nancy"
          },
        {
          "age" : 30,
          "name" : "Amy"
          }
        ]
      },
    {
      "id" : 1,
      "married" : true,
      "name" : "Robert Thomas",
      "sons" : null,
      "daughters" : [
        {
          "age" : 22,
          "name" : "Melissa"
          },
        {
          "age" : 6,
          "name" : "Susan"
          }
        ]
      },
      ...
    ]
  }

I'm only interested in "daughters".
I have mapped the objects into a flat array structure.
The code I used:

var fathers = data.fathers,
      separator = '-------------------',
      persons = []; // collector array

   // Filling the collector array with the
   //   desired data.
   fathers.forEach(function(value) {

     value['daughters'].forEach(function(value) {
       persons.push(value);
     });

   });

It works fine. My complete code here: http://codepen.io/mizech/pen/WrNzJe

But I wonder:
Aren't there better ways (in native JavaScript, no frameworks) then iterating?

Would be pleased if anyone has an idea and would share it.

The operation you want is called flatMap, SelectMany, or concatMap (I might have missed a few names.)

  Map a function over a collection and flatten the result by one-level

As far as I'm aware, this isn't provided out-of-the-box but I found an implementation here:

Array.prototype.flatMap = function(lambda) {
  return Array.prototype.concat.apply([], this.map(lambda));
};

Then you can just write

var persons = data.fathers.flatMap(function(father) { return father.daughters; });

Or if you can use arrow functions,

var persons = data.fathers.flatMap(father => father.daughters);


Random password generator with choice of three strengths My code works fine but I definitely have unecessary repeat code. I'd like to use functions so I don't have repeat code. I would appreciate any advise on how to make my password generator code more concise.

I've tried looking through other forums but I couldn't find the answer I'm looking for.

import string
import random
lowerCase = string.ascii_lowercase[:26]
upperCase = string.ascii_uppercase[:26]
numberList = ["1","2","3","4","5","6","7","8","9"]
specialCharacters = ["*","&","@","!"]
responseList = ["weak","medium","strong"]
randomCharacters =[ ]
i=0

while True:
  choice = input("Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong. ")

  if choice in responseList:
    if choice == "weak":
      while i<=5:
        randomLetterLower = random.choice(lowerCase)
        randomCharacters.append(randomLetterLower)
        i += 1

    elif choice == "medium":
      while i<=4:
        randomLetterLower = random.choice(lowerCase)
        randomCharacters.append(randomLetterLower)
        randomLetterUpper = random.choice(upperCase)
        randomCharacters.append(randomLetterUpper)
        i += 1

    elif choice == "strong":
      while i<=1:
        randomLetterLower = random.choice(lowerCase)
        randomCharacters.append(randomLetterLower)
        randomLetterUpper = random.choice(upperCase)
        randomCharacters.append(randomLetterUpper)
        randomNumber = random.choice(numberList)
        randomCharacters.append(randomNumber)
        randomSpecialCharacter = random.choice(specialCharacters)
        randomCharacters.append(randomSpecialCharacter)
        i += 1

    output = " ".join(randomCharacters)
    print(output)
    i = 0
    randomCharacters =[]

  else:
    break

A flexible and manageable approach would be to create a custom PasswordGenerator which will comprise all functionality for particular password levels. Putting it a separate module, thus it can be imported and used in any place of an application on demand.

The one of possible implementations (considering your conditions):

password_generator.py :

import string
import random

class PasswordGenerator:

    LOWERCASE = string.ascii_lowercase[:26]
    UPPERCASE = string.ascii_uppercase[:26]
    NUMBERS = ["1", "2", "3", "4", "5", "6", "7", "8", "9"]
    SPECIAL_CHARS = ["*", "&", "@", "!"]
    LEVELS = ("weak", "medium", "strong")   # password levels

    @staticmethod
    def _compose_passphrase(iter_num, char_types):
        return ''.join(random.choice(c_type) for i in range(iter_num)
                       for c_type in char_types)

    @classmethod
    def weak(cls):
        iter_num = 6    # number of iterations
        char_types = (cls.LOWERCASE,)
        return cls._compose_passphrase(iter_num, char_types)

    @classmethod
    def medium(cls):
        iter_num = 5
        char_types = (cls.LOWERCASE, cls.UPPERCASE)
        return cls._compose_passphrase(iter_num, char_types)

    @classmethod
    def strong(cls):
        iter_num = 2
        char_types = (cls.LOWERCASE, cls.UPPERCASE, cls.NUMBERS, cls.SPECIAL_CHARS)
        return cls._compose_passphrase(iter_num, char_types)

Client script prompt_password.py :

from password_generator import PasswordGenerator

while True:
    choice = input("Want to create a password? To make your choice for your password strength, "
                   "please respond with one of the following: weak/medium/strong ")

    if choice in PasswordGenerator.LEVELS:
        print(getattr(PasswordGenerator, choice)())   # calling a respective method (password level)
    else:
        break

Running prompt_password.py:

Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong weak
ylwwcj
Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong weak
kktnkx
Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong medium
jLiAoWaHbA
Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong strong
qG8@sM7*
Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong strong
tY8&eL7@
Want to create a password? To make your choice for your password strength, please respond with one of the following: weak/medium/strong best

Process finished with exit code 0


Parsing time ranges with PyParsing The following code is intended to parse a string of the following format and return whether the current time falls in that window:

start_time-stop_time [(day|day-day[, ...])][; ...]

For example,

0900-1200 (mon-wed, sat); 0100-0200 (sun, tue)

The code for parsing that type of string is here:

days = ['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun']
#grammar definition
hour = pp.Regex('[01]\d|2[0-3]')
minute = pp.Regex('[0-5]\d')
miltime = hour.setResultsName('hour') + minute.setResultsName('minute')
dayrange = pp.oneOf(days) + pp.Suppress('-') + pp.oneOf(days)                            #day-day
dayspec = '(' + pp.delimitedList(pp.Group(dayrange | pp.oneOf(days)), delim=',').setResultsName('dayspec') + ')'
window = miltime.setResultsName('start') + '-' + miltime.setResultsName('end') + pp.Optional(dayspec)
spec = pp.StringStart() + pp.delimitedList( pp.Group(window), delim=';' ) + pp.StringEnd()

#parse and act
#return True instead of return condition so that when it fails, it tries the next window
uptime_spec = uptime_spec.strip().lower()
for window in spec.parseString(uptime_spec):
  start = time(int(window.start.hour), int(window.start.minute))
  end = time(int(window.end.hour), int(window.end.minute))
  if start > end:
    raise pp.ParseBaseException('Start time after end time')
  if start < datetime.now().time() < end:
    if not window.dayspec:                #if there's no dayspec, any day is fine
      return True
    else:
      for day in window.dayspec:
        if len(day.asList()) == 1:        #a single day
          if datetime.now().strftime('%a').lower() == day[0]:
            return True
        else:                             #a range
          start = days.index(day[0])
          end = days.index(day[1])
          if start <= end:                #the range doesn't wrap
            if start <= datetime.now().weekday() <= end:
              return True
          else:                           #the range wraps, check that today isn't outside the range
            if not (end < datetime.now().weekday() < start):
              return True

return False

While this solution works, the parsing bit seems rather ugly, especially how I deal with days and day ranges. That part would be easier if I could just test if today's day of the week was in an allowed set of days, and I feel like that might be possible using setParseAction, but I have no idea how to do it.

Any other suggestions? I'm rather new at this parsing thing.

1. Bugs

Supposing that I want to specify an interval of time spanning midnight, for example starting at 23:00 on Monday and ending at 01:00 on Tuesday, how do I do it? The best I can do seems to be:

2300-2359 (mon); 0000-0100 (tue)

but this leaves the minute between 23:59 and midnight on Monday uncovered! One way to fix this would be to allow the input "2400" to represent the midnight at the end of the day.
datetime.now() is called several times during the validation. This could lead to an incorrect result if the code is called at the wrong moment. For example, suppose that my specification is:

1200-1300 (mon); 1100-1200 (mon)

and the function is called just before 12:00 on Monday. When the first window is checked, datetime.now() returns 11:59:59.999, and so the window does not match. But when the second window is checked, datetime.now() returns 12:00:00.001 and so the second window does not match either! But these two windows ought to cover the whole interval from 11:00 to 13:00 on Monday.

If you were to fix the midnight problem in §1.1 above, then there would be a similar race condition in the day handling logic.

So it is necessary to call datetime.now() exactly once and remember the result. Or better still, change the function so that it takes the time to be tested as a parameter.
If the day specification consists of a single day then the code tests for a matching day like this:

datetime.now().strftime('%a').lower() == day[0]

However, strftime is affected by the current locale:

>>> import datetime, locale
>>> locale.setlocale(locale.LC_TIME, 'fr_FR')
>>> datetime.datetime.now().strftime('%a')
'Sam'

so this will go wrong if the locale is not English. It would be more reliable to use:

now.weekday() == days.index(day)

2. Review

The Python style guide (PEP8) recommends four spaces per indentation level, and a maximum of 79 characters per line. You're not obliged to follow this guide, but it'll make it easier to collaborate with other Python programmers if you do. (For example, we wouldn't have to scroll the code horizontally to read it here at Code Review.)
In the pp.delimitedList call, the keyword argument delim defaults to ',' so this can be omitted in the definition of dayspec.
The ParserElement class has an abbreviated syntax for the setResultsName method:

  You can also set results names using the abbreviated syntax, expr("name") in place of expr.setResultsName("name")

Using this would help make the grammar more readable.
The correspondence between grammatical elements and names is inexact. It seems to me that it would make more sense for the hour element to be named 'hour', like this:

hour = pp.Regex('[01]\d|2[0-3]')('hour')
minute = pp.Regex('[0-5]\d')('minute')
time = hour + minute

Assigning a few more grammar elements would make things clearer and help to keep the code within 79 columns, for example:

windows = pp.delimitedList(pp.Group(window), delim=';')
spec = pp.StringStart() + windows + pp.StringEnd()


C# Method Chaining for Email I have been looking into stuff like method chaining, Cascade-Lambda pattern etc.

I have created a class which seems to work fine. However, I just wanted to confirm if this is the best way and the right way to implement this.

For the cascade lambda pattern I was unable to use Action action as a parameter to the Send() method. I couldn't figure out how would I use it.

Any suggestions are welcome.

Here is the code:

public class MailManager
{
    MailMessage _mail { get; set; }
    SmtpClient _smtp { get; set; }

    public MailManager()
    {
        _mail = new MailMessage();
        _smtp = new SmtpClient();
    }

    public MailManager To(string address)
    {
        _mail.From = new MailAddress(address);
        return this;
    }

    public MailManager From(string address)
    {
        _mail.To.Add(new MailAddress(address));
        return this;
    }

    public MailManager Subject(string subject)
    {
        _mail.Subject = subject;
        return this;
    }

    public MailManager Body(string body)
    {
        _mail.Body = body;
        return this;
    }

    public MailManager IsBodyHtml(bool isBodyHtml = true)
    {
        _mail.IsBodyHtml = isBodyHtml;
        return this;
    }

    public MailManager AlternateViews(AlternateView alternateView)
    {
        _mail.AlternateViews.Add(alternateView);
        return this;
    }

    public MailManager Host(string host)
    {
        _smtp.Host = host; return this;
    }

    public MailManager Port(int port)
    {
        _smtp.Port = port; return this;
    }

    public MailManager Credentials(NetworkCredential credentials)
    {
        _smtp.Credentials = credentials; return this;
    }

    public MailManager EnableSsl(bool enableSsl = true)
    {
        _smtp.EnableSsl = enableSsl; return this;
    }

    public void Send()
    {
        using (_smtp)
        {
            _smtp.Send(_mail);
        }
    }
}

This is how I use it:

mailManager.From(_emailSettings.Email)
           .To(item.Email)
           .Subject(string.Format(_emailSettings.Subject, item.Name)).AlternateViews(htmlView)
           .Body(body).IsBodyHtml()
           .Host(_emailSettings.Domain)
           .Port(_emailSettings.Port)
           .Credentials(new NetworkCredential(_emailSettings.Email, _emailSettings.Password))
           .EnableSsl()
           .Send();

As far as method chaining goes, this is neatly implemented.

You've swapped To and From, the method names are inverted. I assume that's just a typo.

public MailManager To(string address)
{
    _mail.From = new MailAddress(address);
    return this;
}

public MailManager From(string address)
{
    _mail.To.Add(new MailAddress(address));
    return this;
}

As it currently stands, I don't think method chaining is really needed here. Every method operates as a basic property setter, which can be handled by using public properties.

This is a matter of preference; but in C# I'd be more inclined to use public properties as opposed to methods.
In the past, all we had was fields and methods. When the tiniest amount of logic was needed to set a field value, we had to rely on methods. Properties were added as a compromise: they are used for setting field values, but you're able to add some minor logic (within reason).

Simple string values can be handled by basic properties:

public string Subject
{
    get { return _mail.Subject; }
    set { _mail.Subject = value; }
}

Note: This example directly sets the value of _mail.Subject. Personally, I'd prefer to not use _mail until the Send() was triggered (and store the values in properties until then), but that may be too subjective for a review.

Your current To() method somewhat conceals the fact that you can add more than one recipient. It doesn't suggest that you're adding to a list. Using a property, this is slightly easier to understand:

public List<string> To { get; set; }

You can cast these into the appropriate MailAddress object and add it to _mail inside the Send() method:

_mail.To.AddRange(this.To.Select(s => new MailAddress(s));

An example of how you'd use a property-based approach:

var mailManager = new MailManager() {
    From = _emailSettings.Email,
    To = new List<string>() { item.Email },
    Subject = string.Format(_emailSettings.Subject, item.Name),
    Body = body,
    IsBodyHtml = true,
    Host = _emailSettings.Domain,
    Port = _emailSettings.Port,
    Credentials =new NetworkCredential(_emailSettings.Email, _emailSettings.Password),
    EnableSsl = true
};

mailManager.Send();

Note: You can also inline everything, i.e; (new MailManager() { ... }).Send();.

Note: I set the property values in the property initializer. You're not required to do so, you can also just store the MailManager in a variable and set its properties when you want to at a later stage (but obviously before calling Send()).

  Should you avoid method chaining?
  No. You wanted to use it, and you used it in a good way. I'm just trying to offer you an alternative which requires less code to achieve the same result.

  If you were implementing method chaining in a class where there is a lot of business logic involved when a method gets calls, then method chaining becomes the preferred approach.
  For the current MailManager class, I find that it doesn't have enough business logic to warrant method chaining just yet. But it's not particularly wrong to do so, just a bit more cumbersome (in my opinion).

Some minor nitpicks:

If you're going to use method chaining to simplify the syntax, it seems consistent to change the Credentials method to take two strings (email,password) instead of a NetworkCredential, or at least create two overloaded methods to allow for either option. Credentials(_emailSettings.Email, _emailSettings.Password) is nicer to read than Credentials(new NetworkCredential(_emailSettings.Email, _emailSettings.Password))
IsBodyHtml() Based on its name, this should be a method that returns a boolean, instead of setting a boolean. A better name would be SetBodyHtml()
AlternateViews(AlternateView alternateView) should really be called AddAlternateView(AlternateView alternateView)
Similarly, To really should be called AddRecipient, since calling the method adds a second value instead of overwriting the first value.

I chose AddRecipient instead of AddTo to avoid semantical confusion between "Add a To-recipient" and "Add this to that".

I would personally change EnableSsl(bool enableSsl = true), though I can see argument for why you used your version.

UseSsl(bool useSsl = true)
EnableSsl() and DisableSsl()


Task Scheduler - LeetCode Challenge I was attempting to solve this problem

  Given a char array representing tasks CPU need to do. It contains
  capital letters A to Z where different letters represent different
  tasks.Tasks could be done without original order. Each task could be
  done in one interval. For each interval, CPU could finish one task or
  just be idle.

  However, there is a non-negative cooling interval n that means between
  two same tasks, there must be at least n intervals that CPU are doing
  different tasks or just be idle.

  You need to return the least number of intervals the CPU will take to
  finish all the given tasks.

  Example 1: Input: tasks = ["A","A","A","B","B","B"], n = 2

  Output: 8

  Explanation: A -> B -> idle -> A -> B -> idle -> A -> B.

The greedy criteria I came up with is, for scheduling a particular task optimally with minimum empty slots we pick the highest occurring task and maintain maximum number of distinct tasks at each stage.

Algoritm :

Load the task by it's occurrence in a queue
While the number of distinct tasks is greater than the cool period we pick the cool number of tasks with highest occurrence and decrease it by 1.
If the number of distinct tasks is less that the cool period then we multiply cool period with the highest occurrence of the task.

The code is below.

template<typename T> int CountLastTask(T& queue, int n)
{
    int count = 0;
    while(!queue.empty() && queue.top().first == n) {
        ++count;
        queue.pop();
    }
    return count;
}

struct compare
{
    bool operator()(const pair<int, char> left, const pair<int, char> right)
    {
        return left.first < right.first;
    }
};
int leastInterval(vector<char>& input, int cool)
{
    map<char, int> values;
    for(auto c : input)
    {
        values[c] += 1;
    }
    int answer = 0;
    priority_queue<pair<int, char>, vector<pair<int, char>>, compare> queue;
    for(auto c: values)
    {
        queue.push({c.second, c.first});
    }
    while(queue.size() > 0)
    {
        auto top = queue.top();

        if(queue.size() < cool + 1)
        {
            int last_task = CountLastTask(queue, top.first);
            answer += ((top.first - 1) * (cool + 1)) + last_task;
            return answer;
        }
        int index = 0;
        vector<pair<int, int>> history;
        while(index < cool + 1)
        {
            top = queue.top();
            if(top.first > 1)
            {
                history.push_back({top.first -1, top.second});
            }
            queue.pop();
            ++index;
        }
        for(auto it : history)
        {
            queue.push(it);
        }
        answer += cool + 1;
    }
    return answer;
}

I am not sure if my solution uses the right data structure in the correct way. As inserting back into the queue requires me to store the pop-ed values in an temporary array and then insert it back.

Could you review this code for the right usage of data structures.

First the nitty picky stuff

Every container has a empty() method. I would consider it bad practice to use  size() > 0 rather than !empty(), as it can have really performance implictations for lists and maps.
Please post complete code. You are missing any headers and also the dubious using namespace std; That one you should really ommit in your code and start using proper namespaces.

So now to the problem at hand. I think your data structure is indeed badly choosen. The first thing to note is what is important.

a. The type of instruction
b. The number of remaining instructions
c. The time until a certain instruction is ready.

You are missing the third point, which complicates the code more than it should. So lets start with a data structure that fits the problem

struct Instruction {
    Instruction(const char id)
        : Identifier(id)
    {}

    const char Identifier;
    size_t remainingUses = 1;
    size_t nextPossibleTime = 0;
}

So I named it next possible time, because the current time since start is the same for every instruction. Now given a current time we can schedule an instruction

char scheduleInstruction(const size_t currentTime, const size_t coolDown) {
    --remainingUses;
    nextPossibleTime = currentTime + coolDown + 1;
    return Identifier;
}

So when we schedule an instruction we decrement the number of remaining uses and set the next possible instruction time until the current time + the cool down period. Obviously we can put currentTime+coolDown+1 in the caller but that is a question of personal preference.

Now to check whether an Instruction can be scheduled we only have to compare currentTime with nextPossibleTime for the instruction.

So now we have scheduled an instruction how do we move it withine a queue? Quite easy there are only 2 relevant quantities

1. nextPossibleTime
2. remainingUses

So lets define a comparison operator for that task as you did

inline bool
operator< (Instruction const& b) const {
    return std::tie(nextPossibleTime, remainingUses) <
           std::tie(b.nextPossibleTime, b.remainingUses);
}

By using std::tie we automatically get the correct comparison of the two fields. Now consider a given container and we have scheduled an instruction, then we swap the elements until our scheduled instruction is not smaller than
another given instruction.

void rescheduleInstructions() {
    auto lastInstruction = myInstructions.rbegin();
    auto nextInstruction = std::next(lastInstruction, 1);

    while(nextInstruction != myInstructions.rend() && (*lastInstruction< *nextInstruction) {
        std::iter_swap(nextInstruction , lastInstruction);
        ++nextInstruction;
        ++lastInstruction;
    }
}

EDIT:

So I made a mistake here. The problem becomes obvious when looking at the simple sequence
AAABC n=2

The A Instruction will be scheduled after B and C with the old code, leading to the sequence

A->B->C->A->Idle->A

However the optial sequence would be

A->B->A->C->A

The reason for the error is, that we only need go back at max coolDown steps as after that point any instruction will be ready and we can sort from there with simply after the remainingUses, which ensures that we always schedule the most important instruction first.

void rescheduleInstructions() {
    auto lastInstruction = myInstructions.rbegin();
    auto nextInstruction = std::next(lastInstruction, 1);

    for (size_t step = 0; step  < coolDown; ++step ) {
        if (nextInstruction == myInstructions.rend()) {
            break;
        }
        if (*nextInstruction < *lastInstruction) {
            break;
        }
        std::iter_swap(nextInstruction , lastInstruction);
        ++nextInstruction;
        ++lastInstruction;
    }
}

So lets wrap this up by creating a class for the instruction set

class InstructionSet {
    InstructionSet (const std::string instructions, const size_t cooldown)
        :coolDown(cooldown)
    {
        for (const char& instruction : instructions) {
            auto pos = std::find(myInstructions.begin(), myInstructions.end(), [&instruction](const Instruction& i) {return i.Identifier == instruction; });
            if (pos != myInstructions.end()) {
                pos->AddInstruction();
            } else {
                myInstructions.emplace_back(instruction);
            }
        }
        std::sort(myInstructions.rbegin(), myInstructions.rend());
    }

    std::string findOptimalSchedule();

private:
    void rescheduleInstructions();

    std::container<Instruction> myInstructions;
    size_t currentTime = 1;
    const size_t coolDown;
}

You could also go for sorted insertion into the vector, however i felt that it complicates the matter of finding and in my experience just inserting and sorting at the end is more performant.

Now for the optimal schedule we have to start with the last element and then go from there. If currentTime is not less than nextPossibleTime of the final instruction we can schedule it. Otherwise due to the ordering we know that there is no possible instruction and we have to go idle (I choose "1" for that). After an instruction has been scheduled, we check whether there are remaining uses and either remove it or reschedule.

    std::string findOptimalSchedule() {
        std::string schedule;
        while(!myInstructions.empty()) {
            auto nextInstruction = myInstructions.rbegin();
            if(currentTime >= nextInstruction->nextPossibleTime) {
                schedule.push_back(nextInstruction->scheduleInstruction(currentTime, coolDown));
                if (nextInstruction->remainingUses == 0) {
                    myInstructions.erase(nextInstruction);
                } else {
                    rescheduleInstructions();
                }
            } else {
                schedule.push_back("1");
            }
            ++currentTime;
        }
        return schedule;
    }

Edit

To put all the code together and remove the unnecessary occurence of std::string although it

#include <algorithm>
#include <vector>

struct Instruction {
    Instruction(const char id)
        : Identifier(id)
    {}

    inline bool
    operator< (Instruction const& b) const {
        return std::tie(nextPossibleTime, remainingUses) <
               std::tie(b.nextPossibleTime, b.remainingUses);
    }

    char scheduleInstruction(const size_t currentTime, const size_t coolDown) {
        --remainingUses;
        nextPossibleTime = currentTime + coolDown + 1;
        return Identifier;
    }

    const char Identifier;
    size_t remainingUses = 1;
    size_t nextPossibleTime = 0;
}

    class InstructionSet {
    InstructionSet (const std::vector<char> instructions, const size_t cooldown)
        :coolDown(cooldown)
    {
        for (const char& instruction : instructions) {
            auto pos = std::find(myInstructions.begin(), myInstructions.end(), [&instruction](const Instruction& i) {return i.Identifier == instruction; });
            if (pos != myInstructions.end()) {
                pos->AddInstruction();
            } else {
                myInstructions.emplace_back(instruction);
            }
        }
        std::sort(myInstructions.rbegin(), myInstructions.rend());
    }

    int findOptimalSchedule() {
        std::vector<char> schedule;
        while(!myInstructions.empty()) {
            auto nextInstruction = myInstructions.rbegin();
            if(currentTime >= nextInstruction->nextPossibleTime) {
                schedule.push_back(nextInstruction->scheduleInstruction(currentTime, coolDown));
                if (nextInstruction->remainingUses == 0) {
                    myInstructions.erase(nextInstruction);
                } else {
                    rescheduleInstructions();
                }
            } else {
                schedule.push_back('1');
            }
            ++currentTime;
        }
        return static_cast<int>(schedule.size());
    }

private:
    void rescheduleInstructions() {
        auto lastInstruction = myInstructions.rbegin();
        auto nextInstruction = std::next(lastInstruction, 1);

        for (size_t step = 0; step  < coolDown; ++step ) {
            if (nextInstruction == myInstructions.rend()) {
                break;
            }
            if (*nextInstruction < *lastInstruction) {
                break;
            }
            std::iter_swap(nextInstruction , lastInstruction);
            ++nextInstruction;
            ++lastInstruction;
        }
    }

    std::vector<Instruction> myInstructions;
    size_t currentTime = 1;
    const size_t coolDown;
}