Helper class to send emails with attachments on Android I use this EmailTools helper class inside an Android app to send data as an email attachment, by calling send(context, deviceName, recordedContent), where the parameters are:

context: an instance of Context, in order to:

create Intent.ACTION_SEND for launching an existing email app to send emails
get elements from string.xml to format the subject
get the app's version to include in the message body

deviceName: a String, the name of the device where the data is coming from.
recordedContent: a String, the recorded data, to send as attachment.

Is there a better way? How would you improve this?

public abstract class EmailTools {

    private static final String TAG = EmailTools.class.getSimpleName();

    private static final String MESSAGE_TYPE = "message/rfc822";

    public static void send(Context context, String deviceName, String recordedContent) {
        String subject = String.format(context.getString(R.string.fmt_subject_recorded_data), deviceName);
        String message = String.format(context.getString(R.string.fmt_recorded_from), deviceName);
        message += getPackageInfo(context);

        Intent intent = new Intent(Intent.ACTION_SEND);
        intent.setType(MESSAGE_TYPE);
        intent.putExtra(Intent.EXTRA_SUBJECT, subject);
        intent.putExtra(Intent.EXTRA_TEXT, message);
        addAttachmentToIntent(context, deviceName, recordedContent, intent);
        launchEmailApp(context, intent);
    }

    private static String getPackageInfo(Context context) {
        String packageName = context.getPackageName();
        PackageManager manager = context.getPackageManager();
        try {
            PackageInfo info;
            if (manager != null) {
                info = manager.getPackageInfo(packageName, 0);
                if (info != null) {
                    return String.format("\n\n--\n[App: %s Version: %d/%s]",
                            packageName, info.versionCode, info.versionName);
                }
            }
        } catch (PackageManager.NameNotFoundException e) {
            Log.e(TAG, "Could not get package info", e);
        }
        return "";
    }

    private static void addAttachmentToIntent(Context context, String deviceName, String recordedContent, Intent intent) {
        SimpleDateFormat dateFormat = new SimpleDateFormat("_yyyyMMdd_HHmm");
        String filename = deviceName + dateFormat.format(new Date()) + ".dat";
        try {
            FileOutputStream ostream = context.openFileOutput(filename, Context.MODE_WORLD_READABLE);
            ostream.write(recordedContent.getBytes());
            ostream.close();
            File attachment = context.getFileStreamPath(filename);
            intent.putExtra(Intent.EXTRA_STREAM, Uri.fromFile(attachment));
        } catch (IOException e) {
            Log.e(TAG, "could not create temp file for attachment :(", e);
        }
    }

    private static void launchEmailApp(Context context, Intent intent) {
        try {
            context.startActivity(Intent.createChooser(intent, context.getString(R.string.email_client_chooser)));
        } catch (ActivityNotFoundException ex) {
            Toast.makeText(context, context.getString(R.string.no_email_client), Toast.LENGTH_SHORT).show();
        }
    }
}

I'd change a few things to make your code cleaner.

Your send method does three different things: it extract the message from the context, it creates the intent and it finally launches the email app. What about introducing a Message class, a Message decodeMessage(Context context, String deviceName), and an Intent createIntent(Message message) methods?

getPackageInfo is definitely to arrow-shaped. I'd refactor it to separate the logic of creation of the package info string String formatPackageInfo(PackageInfo packageInfo) and the code to retrieve the PackageInfo instance.

private PackageInfo getPackageInfo(){
    String packageName = context.getPackageName();
    PackageManager manager = context.getPackageManager();
    if (manager == null)
        return null;
    try
    {
        return manager.getPackageInfo(packageName, 0);
    }
    catch (PackageManager.NameNotFoundException e)
    {
        Log.e(TAG, "Could not get package info", e);
    }
    return null;
}

private static String formatPackageInfo(PackageInfo packageInfo) {
        if(packageInfo == null)
            return "";
        return String.format("\n\n--\n[App: %s Version: %d/%s]",
                        packageName, info.versionCode, info.versionName);
}

In addAttachmentToIntent I'd probably move the declaration of dateFormat and of filename in the try block. You're not going to use them outside that scope so it could be a good idea to give them the smallest scope possible.


Longest common prefix I have made a function for finding the longest common prefix for the challenge on the leetcode site. This is the code:

var longestCommonPrefix = function(strs) {
    let longestPrefix = '';
    if (strs.length > 0) {
      longestPrefix = strs[0];
      for (let i = 1; i < strs.length; i++) {
        for (let j = 0; j < longestPrefix.length; j++) {
          if (strs[i][j] != longestPrefix[j]) {
            longestPrefix = longestPrefix.slice(0, j);
            break;
          }
        }
      }
    }

    return longestPrefix;
};

I am sure there is a way to make this code better, but not sure how to do that. Would appreciate any help.

I would find the alphabetically smallest and largest string and just run your algorithm on these two. That would avoid the embedded loop.

var longestCommonPrefix = function(strs) {
    if (!strs)
        return '';

    let smallest = strs.reduce( (min, str) => min < str ? min : str, strs[0] );
    let largest  = strs.reduce( (min, str) => min > str ? min : str, strs[0] );

    for (let i=0; i<smallest.length; i++) {
        if (smallest[i] != largest[i])
            return smallest.substr(0,i);
    }

    return '';
};

In answer to konijn it would be minimally faster to get the smallest/largest by doing:

let smallest = strs[0];
let largest  = strs[0];
for (let i=1; i<strs.length; i++) {
  let s= strs[i];
  if (s > largest)  largest = s;
  if (s < smallest) smallest = s;
}


Modern C++ Singleton Template I recently read about the C++17 static inline member declaration and thought that this will make templates a little bit cleaner, since static members can now be initialized inside a templated class.

Because of this I wanted to create a neat little Singleton template (since it is the perfect example where static members are needed).

Now to my questions: Is there something I potentially missed, i.e. are there possibilities to create copies of a derived Singleton? Is it a good idea to use CRTP for a Singleton in general? What a about the move constructor, do I need to handle it as well?

Here is the template:

template < typename T >
class Singleton {
  public:
    static T& GetInstance() {
      static MemGuard g; // clean up on program end
      if (!m_instance) {
        m_instance = new T();
      }
      return *m_instance;
    }

    Singleton(const Singleton&) = delete;
    Singleton& operator= (const Singleton) = delete;

  protected:
    Singleton() { };
    virtual ~Singleton() { }

  private:
    inline static T * m_instance = nullptr;

    class MemGuard {
      public:
        ~MemGuard() {
          delete m_instance;
          m_instance = nullptr;
        }
    };
};

And here a possible derived type:

class Test final : public Singleton<Test> {
  friend class Singleton<Test>;
  public:
    void TestIt() { };
  private:
    Test() {}
    ~Test() { /* Test intern clean up */ }
};

Okay so first of the obligatory Singletons are bad practice so you probably shouldn't make it easy to write bad code.

Ignoring the fact that the class probably shouldn't exist at all we can look at the code.

static T& GetInstance() {
  static MemGuard g; // clean up on program end
  if (!m_instance) {
    m_instance = new T();
  }
  return *m_instance;
}

If multiple threads access this instance simultaneously before it is created, you have a data race and m_instance may end up be being constructed multiple times or other kinds of undefined behaviour. You need to add mutex locks around the if block or use std::call_once which is preferred.

As it is supposed to be a singleton you're not supposed to be able to create more instances as the meaning of a singleton is to just have one instance but it appears that it is fully possible to construct multiple instances of Test  simply by creating them as local variables. So this is a design flaw in your template.

A much better way of creating a singleton is to rely on C++11 Magic Statics (N2660). And simply do this:

class Test{
private:
    Test(); // Disallow instantiation outside of the class.
public:
    Test(const Test&) = delete;
    Test& operator=(const Test &) = delete;
    Test(Test &&) = delete;
    Test & operator=(Test &&) = delete;

    static auto& instance(){
        static Test test;
        return test;
    }
};

Which is much easier to write than your code, it's thread safe and fixes the issues with allowing Test to be instantiated. The properties of magic statics guarantee that test will be initialised exactly once the first time the function body is entered by any thread, even in the presence of multiple threads that might otherwise cause a data-race. The instance will be deconstructed when your main() function returns (in the static destruction stage) which makes the whole MemGuard thing unnecessary.


Improving a Haskell FizzBuzz Solution I wrote a solution to the popularized FizzBuzz problem in Haskell to see how a functional solution looks. I'm more or less happy with my solution, except for the definition of list. It just looks sort of contorted.

Are there any language features in Haskell which can improve the readability of this code, especially the definition of list, while preserving the extensibility that table provides?

import Control.Arrow

main = mapM_ (putStrLn . fizzBuzzLogic) [1..100]

fizzBuzzLogic :: Int -> String
fizzBuzzLogic x
  | null list = show x
  | otherwise = foldl1 (++) list
  where
    list  = map snd . filter ((==0) .fst) $ map (first (mod x)) table
    table = [(3,"Fizz")
            ,(5,"Buzz")
            ] --add more modulo tokens if you wish

You write:

map snd . filter ((==0) .fst) $ map (first (mod x)) table

To make it clearer that we have three operations applied one after another, I would rather write:

map snd . filter ((==0) .fst) . map (first (mod x)) $ table

But I think this gets easier to read if we merge the filter and the second map:

map snd . filter ((== 0) . mod x . fst) $ table

Some people prefer pointful style, that is, they prefer \ over .:

map snd . filter (\(number, text) -> x `mod` number == 0) $ table

Personally, I would prefer a list comprehension here:

[text | (number, text) <- table, x `mod` number == 0]


Read uncommited rows and lock row on update I have 5 (or more) instances of an application running on same or different machines. Each of these application will pass a comma separated string of UserIDs and the app should pick up a unique unused record (UserID) from a DB table, matching the user IDs as passed through the SP.
So if App-1 picks User-3 and marks it as 'locked' then User-3 will not be available for other apps until, App-1 remarks it as 'unlocked'.

I have written a small SP to mark the first 'unlocked' user as 'locked' in the DB table and return the UserID to the calling app.

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:      Souvik Ghosh
-- Create date: 12-Nov-2016
-- Description: Lock a user ID and return it
-- =============================================
ALTER PROCEDURE LockHROSAPID
@UserIDs varchar(max)
AS
BEGIN
    SET NOCOUNT ON;
    BEGIN TRY
        BEGIN TRANSACTION
            UPDATE [wm_uc1].[UK_HR_HROSAPUserIDs] SET IsLocked = 1
            OUTPUT Inserted.HROSAPUID
            WHERE ID = (SELECT TOP 1 ID FROM [wm_uc1].[UK_HR_HROSAPUserIDs]
            WHERE HROSAPUID IN (@UserIDs) AND IsLocked IS NULL OR IsLocked <> 1)
        COMMIT TRANSACTION
    END TRY
    BEGIN CATCH
        ROLLBACK
    END CATCH
    RETURN
END
GO

I have tested it and it seems to work fine. But, I want to have a second opinion about this.

Some problems here

OR IsLocked <> 1 will be process last
SELECT TOP 1 ID is non-deterministic
I hope HROSAPUID has a unique constraint

_

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:      Souvik Ghosh
-- Create date: 12-Nov-2016
-- Description: Lock a user ID and return it
-- =============================================
ALTER PROCEDURE LockHROSAPID
@UserIDs varchar(max)
AS
BEGIN
    SET NOCOUNT ON;
    BEGIN TRY
        BEGIN TRANSACTION
            UPDATE [wm_uc1].[UK_HR_HROSAPUserIDs]
            SET IsLocked = 1
            OUTPUT Inserted.HROSAPUID
            WHERE ID = ( SELECT TOP 1 ID
                           FROM [wm_uc1].[UK_HR_HROSAPUserIDs]
                          WHERE HROSAPUID IN (@UserIDs)
                            AND IsLocked IS NULL
                             OR IsLocked <> 1 )
        COMMIT TRANSACTION
    END TRY
    BEGIN CATCH
        ROLLBACK
    END CATCH
    RETURN
END
GO

maybe something more like this
a single statement is a transaction

  update top (1) [Table_1]
  set bbb = 'no'
  output inserted.pk
  from [Table_1]
  where ( bbb <> 'no' or bbb is null )
  and pk in (13,14,15)


Bowling game Kata in Scala - pattern match This is a functional approach to problem described by standard Bowling Game Kata - to implement the rules of calculating a score in a bowling game. The inspiration was the implementation by Uncle Bob: http://butunclebob.com/ArticleS.UncleBob.TheBowlingGameKata

  The game consists of 10 frames as shown above.  In each frame the
  player has two opportunities to knock down 10 pins.  The score for the
  frame is the total number of pins knocked down, plus bonuses for
  strikes and spares.

  A spare is when the player knocks down all 10 pins in two tries.  The
  bonus for that frame is the number of pins knocked down by the next
  roll.

  A strike is when the player knocks down all 10 pins on his first try.
  The bonus for that frame is the value of the next two balls rolled.

  In the tenth frame a player who rolls a spare or strike is allowed to
  roll the extra balls to complete the frame.  However no more than
  three balls can be rolled in tenth frame.

Any feedback of my pattern-match-with-recursion implementation f bowling game kata will be greatly appreciated:

import org.scalatest.{FlatSpec, Matchers}

class BowlingGameTest extends FlatSpec with Matchers {
  "sanity test" should "pass" in {
    true shouldBe true
  }

  "gutter game" should "have no points" in {
    val score = BowlingGame.count(Seq.fill(20)(0))
    score shouldBe 0
  }

  "all ones" should "return sum of points" in {
    val score = BowlingGame.count(Seq.fill(20)(1))
    score shouldBe 20
  }

  "spare" should "get bonus from next throw" in {
    val points: Seq[Int] = Seq(5, 5, 8, 0) ++ Seq.fill(16)(0)
    val score = BowlingGame.count(points)
    score shouldBe 26
  }

  "multiple spares" should "get bonus from each next throw" in {
    val points: Seq[Int] = Seq(5, 5, 8, 2, 7) ++ Seq.fill(15)(0)
    val score = BowlingGame.count(points)
    score shouldBe 42
  }

  "strike" should "get bonus points from two next throws" in {
    val points: Seq[Int] = Seq(10, 5, 2, 0) ++ Seq.fill(16)(0)
    val score = BowlingGame.count(points)
    score shouldBe 24
  }

  "multiple strikes" should "get bonus points from two next throws" in {
    val points: Seq[Int] = Seq(10, 10, 5, 2, 0) ++ Seq.fill(15)(0)
    val score = BowlingGame.count(points)
    score shouldBe 49
  }

  "spare in last round" should "get bonus points from a single additional throw" in {
    val points: Seq[Int] = Seq.fill(18)(0) ++ Seq(5, 5, 8)
    val score = BowlingGame.count(points)
    score shouldBe 18
  }

  "strike in last round" should "get bonus points from two additional throws" in {
    val points: Seq[Int] = Seq.fill(18)(0) ++ Seq(10, 5, 3)
    val score = BowlingGame.count(points)
    score shouldBe 18
  }

  "strike in last round and in first additional throw" should "get no bonus points from additional throws" in {
    val points: Seq[Int] = Seq.fill(18)(0) ++ Seq(10, 10, 3)
    val score = BowlingGame.count(points)
    score shouldBe 23
  }

  "maximum game" should "get bonuses in each round" in {
    val score=  BowlingGame.count(Seq.fill(12)(10))
    score shouldBe 300
  }

  "partial game" should "return score so far" in {
    BowlingGame.count(Seq(1,2,3)) shouldBe 6
  }
}

object BowlingGame {
  def count(points: Seq[Int]): Int = {
    def countHelper(ps: Seq[Int], score: Int): Int = ps match {
      case Nil => score
      case left :: Nil => score + left
      case lastRoundStrike :: additional1 :: additional2 :: Nil => score + lastRoundStrike + additional1 + additional2
      case strike :: next :: nextnext :: _ if strike == 10 => countHelper(ps.tail, score + strike + next + nextnext)
      case spareL :: spareR :: next :: pss if spareL + spareR == 10 => countHelper(next :: pss, score + spareL + spareR + next)
      case left :: right :: pss => countHelper(pss, score + left + right)
    }

    countHelper(points, 0)
  }

}

Some tips:

When pattern matching and inspecting multiple values of a sequence type (Seq, List, Array, etc) you can reduce the length of your expression by using sequence patterns instead of explicitly breaking the sequence up with the :: operator. So instead of a :: b :: c :: d :: Nil you can more concisely use Seq(a, b, c, d).

When you want to check for explicit values while pattern matching on an object you can use those values instead of replacing them with variables that are then checked with an if statement. For example case Seq(a, b, c) if a == 10 => ...  can be more concisely expressed as case Seq(10, b, c) => ...

When using pattern matching on sequences (especially when the case expression goes beyond the standard head :: tail) it can enhance legibility to use single letter variable names and comments instead of long variable names. I think that we would both agree that good code should describe itself through both value and function names. However, in this case there is no way of adding intuition with function names and so all of the description ended up in your value names.
Like I said before, do what works for you. If legibility was our only metric then maybe pattern matching isn't the ideal way to implement your function. It could still be recursive, but instead of cases you could use functions. Do whatever suits you the best but when possible I would recommend short variable names accompanied by comments over long variable names.

Some code:
object BowlingGame {
  def count(points: Seq[Int]): Int = {
    def helper(ps: Seq[Int], acc: Int): Int = ps match {
      case Nil =>
        acc

      case Seq(a) =>
        acc + a

      // strike or spare in last round
      case xs @ Seq(a, b, c) =>
        acc + xs.sum

      // strike
      case Seq(10, a, b, _*) =>
        helper(ps.tail, acc + 10 + a + b)

      // spare
      case Seq(a, b, c, _*) if a + b == 10 =>
        helper(ps.drop(2), acc + a + b + c)

      // neither a spare or a strike
      case Seq(a, b, _*) =>
        helper(ps.drop(2), acc + a + b)
    }
    helper(points, 0)
  }
}


Creating a scraper using multithreading I've written a script in python using "threading" module to scrape two sites simultaneously. It parses the two sites flawlessly. Any insight as to how I can improve this script will be appreciated.

Here is what I did:

import requests ; from lxml import html
import threading ; import time

Yp_link = "https://www.yellowpages.com/search?search_terms=coffee&geo_location_terms=Los%20Angeles%2C%20CA&page=2"
Tuts_link = "http://www.wiseowl.co.uk/videos/"

def create_links(url):
    response = requests.get(url).text
    tree = html.fromstring(response)
    for title in tree.cssselect("div.info"):
        name = title.cssselect("a.business-name span[itemprop=name]")[0].text
        street = title.cssselect("span.street-address")[0].text
        phone = title.cssselect("div[itemprop=telephone]")[0].text if title.cssselect("div[itemprop=telephone]") else ""
        time.sleep(1)
        print(name, street, phone)

def process_links(link):
    response = requests.get(link).text
    tree = html.fromstring(response)
    for titles in tree.xpath("//p[@class='woVideoListDefaultSeriesTitle']"):
        title = titles.xpath('.//a')[0]
        time.sleep(1)
        print(title.text, title.attrib['href'])

th1 = threading.Thread(target=create_links, args=(Yp_link,))
th2 = threading.Thread(target=process_links, args=(Tuts_link,))

th1.start()
th2.start()

th1.join()
th2.join()

First of all, I think you are putting time.sleep() calls into the wrong places - you are putting them into the loops where you iterate over the extracted elements. Elements are already extracted and no requests are issued at that point - add delays between each requests - at the end of your functions.

I would also improve naming - Yp_link and Tuts_link can be renamed to more explicit YELLOW_PAGES_URL and WISEOWL_URL - note that I think these two need to be defined as proper constants - in upper case.

And, I would also switch to CSS selector locators for process_links() function as well.

As far as imports go, just don't put them on same lines - put each import on it's own line as per PEP8 importing guidelines.


TCP communication with step motor I have a class that I use to communicate with some hardware equipment (a step motor to be precise):

public class HdCommunicator : IDisposable
{
    private TcpClient client;
    public HdCommunicator(TcpClient _client)
    {
        client = _client;

        //create and start the listener
        Thread t = new Thread(new ThreadStart(() => { Listen(); }));
        t.Start();
    }

    public void SendCommand(int data)
    {
        NetworkStream ns = client.GetStream();
        var msgToSend =
            Encoding.UTF8.GetBytes("mr " + data + ",1" + Environment.NewLine);
        ns.Write(msgToSend, 0, msgToSend.Length);

        ns.Flush();
    }

    private void Listen()
    {
        while (client.Connected)
        {
            try
            {
                NetworkStream ns = client.GetStream();

                byte[] bytes = new byte[1024];
                int bytesRead = ns.Read(bytes, 0, bytes.Length);

                string response = Encoding.ASCII.GetString(bytes, 0, bytesRead);

                //TODO raise event
                Console.WriteLine("Response: " + response);
            }
            catch (Exception ex)
            {
                if (!client.Connected)
                { /*ignore*/ }
                else
                {
                    Console.WriteLine("Listener error: " + ex.Message);
                }
            }
        }
    }

    public void Dispose()
    {
        client.Close();
    }
}

And this is how I currently use it:

    static int port = 503;
    static string host = "192.168.33.1";

    static void Main(string[] args)
    {
        HdCommunicator comm = new HdCommunicator(new TcpClient(host, port));

        string res = "";
        while (res != "e")
        {
            Console.WriteLine("Where to send: ");
            res = Console.ReadLine();

            if (res != "e")
                comm.SendCommand(int.Parse(res));
        }

        comm.Dispose();

        Console.WriteLine("Finished");
        Console.ReadLine();
    }

Am I utilizing the TCP Client correctly? I'm particularly concerned about the streams' usage and disposing of them. Am I doing it right?

I'm afraid that I don't know anything about TCP, but I did notice this.

  if (!client.Connected)
  { /*ignore*/ }
  else
  {
     Console.WriteLine("Listener error: " + ex.Message);
  }

Instead of checking for the negative condition, check for the positive. It removes several lines of code and a comment.

if(client.Connected)
{
    Console.WriteLine("Listener error: " + ex.Message);
}


You're logged in, except it's an error I've automated downloading of data from a 3rd party website. The process involves logging into the site, downloading the data, then logging out - lather, rinse, repeat for each client.

The code is in VBA using Access as the host and Selenium Basic to handle the browser details. Hate all you want, but I've implemented it as a WebDownloader interface with multiple implementations (one for each of the sites I have to download from). Plus, when the only tool you have is a stone, everything looks like a nail....

Occasionally we can't get logged into the system for one or more of a variety of reasons (like our AP department is very slow about paying bills and a site will shut us off...).

The following code handles the login process. Prior to this, the browser is opened and the website's login page is retrieved. Once I've gotten to this point, it's safe to assume that everything is ready for the login process to begin.

This works, but it's ugly, and my concern is that the normal, expected course of action goes through the error handler.

Private Sub IWebDownloader_LoginToWebPage()

  Const USER_NAME_FIELD_ID As String = "login"
  Const PASSWORD_FIELD_ID As String = "password"
  Const LOGIN_BUTTON_CLASS_ID As String = "btn-form"

  LogManager.Log TraceLevel, "ApptPlusWebDownloader.IWebDownloader_LoginWebPage"

  On Error GoTo ErrorHandler
  With this.Driver
    .FindElementById(USER_NAME_FIELD_ID).SendKeys this.UserID
    .FindElementById(PASSWORD_FIELD_ID).SendKeys this.Password
    .FindElementByClass(LOGIN_BUTTON_CLASS_ID).Click
  End With
  Dim noteElement As WebElement
  Set noteElement = this.Driver.FindElementByClass("note")
  'assumes we find a note which, normally, we shouldn't
  this.IsLoggedIn = False
  If InStr(1, noteElement.text, "disabled") Then
    LogManager.Log WarnLevel, "Login disabled --- ApptPlus message: " & noteElement.text
  Else
    LogManager.Log WarnLevel, "Login warning --- ApptPlus message: " & noteElement.text
  End If

CleanExit:
  Exit Sub

ErrorHandler:
  With Err
    If .Number = 7 Then
      this.IsLoggedIn = True
    Else
      LogManager.Log ErrorLevel, "ApptPlus Failed to login. UserID: " & this.UserID
      this.IsLoggedIn = False
    End If
  End With
  Resume CleanExit

End Sub

The only way to determine if there's a login issue is to look for the note element appearing on the login page - failure to find that element is expected, preferred and is a good thing.

I could resolve this by wrapping with an OERN like this:

On Error Resume Next
Set noteElement = this.Driver.FindElementByClass("note")
'handle the expected error which is really a good thing here
On Error GoTo ErrorHandler

Is that the best way to go about this, or does someone see something better I could do?

I'm also open to any other tips or suggestions anyone sees that could improve the code.

Abstract it away!

Not sure exactly which instruction means to throw error 7 (which is normally an out of memory error code), but I think something like this should work - basically the idea is to extract the piece of logic into its own self-contained function, so that the calling code doesn't need to worry about an expected error and can use a simple Boolean value instead:

Private Function IsLoginEnabled(Optional ByRef outNoteElement As WebElement) As Boolean
    On Error Resume Next
        Set outNoteElement = this.Driver.FindElementByClass("note")
        IsLoginEnabled = outNoteElement Is Nothing And Err.Number = 7
        Err.Clear
    On Error GoTo 0
End Function

The calling code's logic can now leverage a simple Boolean result:

this.IsLoggedIn = IsLoginEnabled(noteElement)

It can also see that noteElement only ever contanis a valid object reference when the function returns False, so you don't need to be dealing with a run-time error in order to determine whether you need to inspect that noteElement object give your AP department a call.


Generating playing cards This code has several functions and procedures for the program to generate cards like '5 of diamond'. I would like to shorten the code.

def random(one,two):
    import random
    number = random.randint(one,two)
    return number

def suit():
    suitnumber = random(1,4)
    if suitnumber == 1:
        suitb = 'Spade'
    elif suitnumber == 2:
        suitb = 'Hearts'
    elif suitnumber == 3:
        suitb = 'Diamonds'
    elif suitnumber == 4:
        suitb = 'Clubs'

    return suitb

def number():
    number = random(1,13)
    if number == 13:
        value = 'Ace'
    elif number == 12:
        value = 'King'
    elif number == 11:
        value = 'Queen'
    elif number == 10:
        value = 'Jack'
    elif number < 10:
        value = number

    return value

def card():
    cardnumber = number()
    cardsuit = suit()
    card = cardsuit,cardnumber
    return card

def store10Cards():
    tenCards = [card(),
               card(),
               card(),
               card(),
               card(),
               card(),
               card(),
               card(),
               card(),
               card()]
    return tenCards

def yourCards():
    cards = store10Cards()
    counter = 1
    choice = int(input('Which of your cards would you want to see? You have 10 cards : '))
    if choice == 1:
        print('Your chosen card is',cards[0])
    elif choice == 2:
        print('Your chosen card is',cards[1])
    elif choice == 3:
        print('Your chosen card is',cards[2])
    elif choice == 4:
        print('Your chosen card is',cards[3])
    elif choice == 5:
        print('Your chosen card is',cards[4])
    elif choice == 6:
        print('Your chosen card is',cards[5])
    elif choice == 7:
        print('Your chosen card is',cards[6])
    elif choice == 8:
        print('Your chosen card is',cards[7])
    elif choice == 9:
        print('Your chosen card is',cards[8])
    elif choice == 10:
        print('Your chosen card is',cards[9])

        import time
        time.sleep(5)

    return cards

print('Hi')
import time
time.sleep(2)
print('You have 10 cards')
time.sleep(2)
choice = input('Would you like to see them? Y/N  : ')
choice = choice.title()
if choice == 'Y':
    yourCards()
elif choice == 'N':
    print('Too bad')
    time.sleep(1)
    import sys
    sys.exit(0)

You should collect all imports at the beginning of the file. You have strange card numbers and why is choice 10 special? Normally, card games have decks, and not random constructed cards, e.g. you could have 10 equal cards.

import time
import random

SUITS = ('Spade', 'Hearts', 'Diamonds', 'Clubs')
NUMBERS = (1,2,3,4,5,6,7,8,9,10,'Jack','Queen','King','Ace')

def card():
    return random.choice(SUITS), random.choice(NUMBERS)

def store10Cards():
    return [card() for _ in range(10)]

def yourCards():
    cards = store10Cards()
    choice = int(input('Which of your cards would you want to see? You have 10 cards : '))
    print('Your chosen card is', cards[choice - 1])
    if choice == 10:
        time.sleep(5)
    return cards

print('Hi')
time.sleep(2)
print('You have 10 cards')
time.sleep(2)
choice = input('Would you like to see them? Y/N  : ').upper()
if choice == 'Y':
    yourCards()
elif choice == 'N':
    print('Too bad')
    time.sleep(1)


A wounded Zombie finder, or: how to find the almost dead undead? The Zombies are on their way to eat our statistics. Fortunately, we have a way to get rid of them: our votes. However, we need to find them first. The /unanswered section is too large and contains many strong enemies where we have to write a review first. But the StackExchange Data Explorer can sniff almost dead Zombies from a mile.

For this, we need a fitting query. This is what I've tried:

SELECT
  q.Id AS [Post Link],
  MAX(a.Score) AS [Best answer score]
FROM
  Posts q
JOIN
  Posts a ON a.ParentId = q.Id
WHERE
      q.PostTypeId = 1
  AND q.AnswerCount > 0
  AND q.AcceptedAnswerId IS NULL
  AND a.PostTypeId = 2
GROUP BY
  q.Id
HAVING
 MAX(a.Score) = 0

You can run and find the query on SEDE.

It searches for questions (q.PostTypeId = 1) that have at least one answer, no accepted answer, and the maximum score of all its answers is 0. The resulting list can then be checked for fast Zombie removal.

Is there something that is utterly wrong? Or can this weapon help us in our fight against the undead?

Not utterly wrong, but

q.PostTypeId = 1

and

AND a.PostTypeId = 2

are superfluous. The only post types with a non-null value in ParentId are answers (see the Database schema documentation), and their values are the ids of questions, so the join already takes care of this.

I'd slightly misunderstood the objective of the query at first, because questions with answers with negative scores, like this one also appear on /unanswered. But you want to review the answers to see if they're worthy of an upvote. Then, you might as well drop the second column (MAX(a.Score) AS [Best answer score]) because you know it's going to be zero.


Vectorising orthogonal-triangular decomposition for 3D matrix I am trying to optimise the computation time of my code (for the second time after a first optimisation that gives very good results). Currently, this code is very time-consuming depending on the size of the data (i.e. sometimes very big matrices). So if anyone has some ideas to help me to optimize this code, I will be very grateful!

matrice1=0+(20-0).*rand(500,5);
matrice2=datasample(matrice1,100);

[n1,~]=size(matrice1);
[n2,p2]=size(matrice2);
% Pre-calculate replicated B and the indices to be modified at each iteration
B_rep=repmat(matrice2,[1 1 n2]);
B_idx=bsxfun(@plus,((0:p2-1)*n2+1)',(0:n2-1)*(n2*p2+1));
B=mean(B_rep,1);
out=zeros(n1,n2); % initialize output array
for i=1:n1
    B_rep(B_idx)=repmat(matrice1(i,:)',1,n2);
    B_meanB=bsxfun(@minus,B_rep,B); % B minus mean values of B
    A_B_meanB=matrice2'-reshape(B,p2,[]); % A minus B_meanB

    for j=1:n2
        [~,R]=qr(B_meanB(:,:,j),0);
        out(i,j)=sum((R'\A_B_meanB(:,j)).^2)*(n2-1);
    end
end

Your code looks fairly good, but there are a few things I would do differently:

It's very confusing that you call B_rep for B, and call B the mean of B_rep. The comment and code here looks very strange. You should call B_mean = mean(B_rep, 1) to stick with the general naming convention in your code.

B_meanB=bsxfun(@minus,B_rep,B); % B minus mean values of B

bsxfun performs better than repmat, so instead of B_rep=repmat(matrice2,[1 1 n2]); you can do:

B_rep = bsxfun(@times, matrice2, ones(1,1,n2));

Instead of ', you should use .' when transposing an array. The first one is the complex conjugated transpose.
i and j are bad variable names in Matlab.
You should try to use more spaces. It makes the code much easier to read.

I don't have the statistical toolbox, so I can't test your code myself. I'm not sure how it can be vectorized, so I can't help with much when it comes performance gain I'm afraid.


Implementing Tribool with int8_t For digital electronic circuit simulation, i wanted to implement Three states. i.e High, Low and Undefined in C++.

I saw Boost::tribool, and they implement it using enum

Now for conserving some memory i have implemented it using 1 Byte Int

#ifdef _WINDOWS
typedef __int8 State;
#elif
typedef int8_t State;
#endif

const State kLow = 0;
const State kHigh = 1;
const State kUndefined = 2;

Do you see any potential problem with it?
And why would one waste 4 bytes instead of a single byte for 3 values only
Do you have a still better way?

I would definitely recommend using enumerations. As this is December 2011, I would imagine your compiler supports an explicit underlying type on enumerations (GCC supported this since 4.4 and Visual C++ now supports it in 11.0). As a side note, <cstdint> is supported by visual c++ now (as of 10.0).

As an addenum to traditional c-style enumerations:

enum state : std::int8_t {
    S_UNKNWON,
    S_HIGH,
    S_LOW
};

Or adding scope to the enumeration's memebers (avoid polluting global scope):

enum class state : std::int8_t {
    unknown,
    high,
    low
};

state status = state::unknown;

Of course, after looking at boost::tribool, I would recommend that over anything -- unless you really need to ensure that state is exactly 1 byte.


"Flatten dictionary" challenge I want to ask for code review for my JavaScript implementation of Flatten_dictionary about this coding challenge for "Flatten a Dictionary" can be found at pramp:

  Given a dictionary dict, write a function flattenDictionary that
  returns a flattened version of it .

  If you’re using a compiled language such Java, C++, C#, Swift and Go,
  you may want to use a Map/Dictionary/Hash Table that maps strings
  (keys) to a generic type (e.g. Object in Java, AnyObject in Swift
  etc.) to allow nested dictionaries.

  Example:

  Input:

dict = {
    "Key1" : "1",
    "Key2" : {
        "a" : "2",
        "b" : "3",
        "c" : {
            "d" : "3",
            "e" : "1"
        }
    }
}

  Output:

{
    "Key1" : "1",
    "Key2.a" : "2",
    "Key2.b" : "3",
    "Key2.c.d" : "3",
    "Key2.c.e" : "1"
}

  Important: when you concatenate keys, make sure to add the dot character between them. For instance concatenating Key2, c and d the result key would be Key2.c.d.

#implementation#1

function flattenDictionary(dict) {
 //flattened dictionary = {}
  //iterate through dictionary
  //check for type of value for each key
     //if value is another dictionary, i will recurivec callthis appending it to the dictionary
  let flattened = {}
  let keys = Object.keys(dict) //give me an array of keys
  for (let i=0; i< keys.length; i++){
    if (typeof dict[keys[i]] === "object") {
      let flatObj = flattenDictionary(dict[keys[i]])
      let newKeys = Object.keys(flatObj) //[a,b]
      for (let j=0; j<newKeys.length; j++) {
        if (newKeys[j] === "") {
          flattened[keys[i]] = flatObj[newKeys[j]]
        } else {
           flattened[keys[i]+"."+newKeys[j]] = flatObj[newKeys[j]]
        }

      }
      //get back to this
    } else {
      flattened[keys[i]] = dict[keys[i]]
    }
  }
  return flattened
}

#implementation#2

function flattenDictionary(dict) {
  // your code goes here
  let results = {};
  let currKey = "";

  function flattenUtil(dict) {
    // 1. 'Iterate' over the dictionary
    // Keys will either be 'Key_' for outermost level
    // or a letter/empty string for inner levels
    // 2. If the value is an int/string, append onto output dictionary
    // else if the value is a dictionary, recurse
    // 3. Base case: dict empty, return

    Object.keys(dict).forEach(key => {
      if (typeof dict[key] === 'object') {
        if (!currKey) {
          currKey += key;
        } else {
          currKey += `.${key}`;
        }
        flattenUtil(dict[key]);
      } else {
        if (!currKey) {
          results[`${key}`] = dict[key];
        } else {
          if (!key) {
            results[`${currKey}`] = dict[key];
          } else {
            results[`${currKey}.${key}`] = dict[key];
          }
        }
      }
    });
  }

  flattenUtil(dict);
  return results;
}

One issue I always come across when implementing this myself is that keys can contain .. But your code simply concatenates nested keys together, your flattener function will not correctly represent the object it just flattened. You'll have to find a way to tell a valid . from a . that was part of the key.

While Object.keys() is a really handy function to get the object keys, you're effectively creating an array with it. Same goes for forEach's callback, you're creating the callback function. For larger objects, you easily generate lots of objects this way. This can cause the GC to kick in more frequently, causing things like jerky scrolling, UI freezing, animation hitching. I recommend using plain for-in loops once you start seeing these issues.

Also, your code overcomplicated itself. At every node, your function is generating a flat object, and its return is merged to the parent node's flat object. Theoretically, this is how things are done in a "functional" manner - create, never mutate. But this becomes very impractical, you're building throw-away objects. What you could do is create just one object. Then as you traverse through the tree, just append to it. To the outside world, the function is still pure even if its implementation isn't.

For instance, your code could have been as simple as this (ignoring the . in key issue):

const dict = {
    "Key1" : "1",
    "Key2" : {
        "a" : "2",
        "b" : "3",
        "c" : {
            "d" : "3",
            "e" : "1"
        }
    }
}

const flatten = (source, parentPath = '', target = {}) => {

  for(const key in source){
    // Construct the necessary pieces of metadata
    const value = source[key]
    const path = parentPath ? `${parentPath}.${key}` : key

    // Either append or dive another level
    if (typeof value === 'object'){
      flatten(value, path, target)
    } else {
      target[path] = source[key]
    }
  }

  return target
}

// To the outside world, you pass in dict, and you get a new object.
// No side effects happen, we're all good.
console.log(flatten(dict))


Review of linked list and quick sort I was trying another approach of sorting a linked list. Aside from the available methods, I've decided to take each node from the linked list and place it into an array.  With that, I would be able to compare the data variables easily. I applied quickSort() on the array, and this is what I have.  I would appreciate any feedback/comments on my code.

    public static void main(String[] args) {

        MyLinkedList list = new MyLinkedList();

        Student s = new Student(1, "John", 20, "Italy", "2011");
        list.addStudent(s);
        Student s2 = new Student(2, "Mark", 19, "UAE", "2010");
        list.addStudent(s2);
        Student s3 = new Student(3, "Sally", 35, "UAE", "2000");
        list.addStudent(s3);

        System.out.println("Students in the list: ");
        list.print();

        Node[] n = list.convertA(list);
        quickSort(n, 0, (n.length-1));
        System.out.println("Sorted list is:");

        for(int q =0;q<n.length;q++){
            System.out.println(n[q] + " ");
        }
    }

    public static int partition(Node arr[], int left, int right) {

        int i = left, j = right;

        Node tmp;

        Node pivot = arr[(left + right) / 2];

        while (i <= j) {

            while (arr[i].getStudent().getAge() < pivot.getStudent().getAge()) {
                i++;
            }

            while (arr[j].getStudent().getAge() > pivot.getStudent().getAge()) {
                j--;
            }

            if (i <= j) {

                tmp = arr[i];
                arr[i] = arr[j];
                arr[j] = tmp;
                i++;
                j--;

            }

        }
        return i;

    }

    public static void quickSort(Node arr[], int left, int right) {

        int index = partition(arr, left, right-1);

        if (left < index - 1) {
            quickSort(arr, left, index - 1);
        }

        if (index < right) {
            quickSort(arr, index, right);
        }
    }

First of all I don't know why are you trying to reinvent the wheel? Java has a strong and beautiful API with many methods and there is also a method for sorting an custom array. Which is more faster than quicksort. However I think this is for your own practice.

Your convertA method is not good, cause you are passing the reference as argument as well. So either make the method static or use the existing method toArray().
You are converting a student list to an Node array, why? change Node array to Student array. So this

arr[i].getStudent().getAge()

will change to

arr[i].getAge()

there is a possible bug you are calculating (left + right) / 2 which may cause an overflow, though Java int range is very big you will face the overflow very rarely, but if you want to fix this; change to (right - left) / 2 + left.

Check How Other Have Done The Quicksort :

From CodeReview 1

From CodeReview 2

Why you should use the API method?


Making ArrayList.containsAll run faster Whenever using ArrayList, the method containsAll(collection) runs in time \$\mathcal{O}(mn)\$, where \$n\$ is the size of ArrayList and \$m\$ is the length of collection. However, it relies only on equals(Object)-method.

I have this subclass of ArrayList that does the same in time \$\mathcal{O}(n)\$, but relies on hashCode() as well.

My main question, should I do this optimization at expense of adding more requirements?

MyArrayList.java:

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Random;
import java.util.Set;

/**
 * This class provides a faster {@link #containsAll(java.util.Collection) }
 * method.
 *
 * @author Rodion "rodde" Efremov
 * @version 1.6
 * @param <E> the actual element type.
 */
public class MyArrayList<E> extends ArrayList<E> {

    /**
     * Returns {@code true} if this list contains all elements in {@code coll}
     * and {@code false} otherwise.
     *
     * @param  coll the collection to check for inclusion in this list.
     * @return {@code true} if this list contains all elements in
     *         {@code coll}:
     */
    @Override
    public boolean containsAll(Collection<?> coll) {
        Set<?> set = new HashSet<>(coll);

        for (E element : this) {
            if (set.contains(element)) {
                set.remove(element);

                if (set.isEmpty()) {
                    return true;
                }
            }
        }

        return false;
    }

    public static void main(final String... args) {
        List<Integer> jdkList = new ArrayList<>();
        List<Integer> myList  = new MyArrayList<>();

        for (int i = 0; i < 1000000; ++i) {
            jdkList.add(i);
            myList.add(i);
        }

        Random rnd = new Random();
        Collection<Integer> coll = new ArrayList<>();

        for (int i = 0; i < 1000; ++i) {
            coll.add(rnd.nextInt(myList.size()));
        }

        // Uncomment for failing test.
        //coll.add(-1);

        long ta = System.currentTimeMillis();

        boolean result = jdkList.containsAll(coll);

        long tb = System.currentTimeMillis();

        System.out.println("ArrayList.containsAll() in " + (tb - ta) + " ms. " +
                           "Result: " + result);

        ta = System.currentTimeMillis();

        result = myList.containsAll(coll);

        tb = System.currentTimeMillis();

        System.out.println("MyArrayList.containsAll() in " + (tb - ta) +
                           " ms. Result: " + result);
    }
}

Relying on .hashCode() may or may not be faster and may or may not produce the same results as relying on .equals().

Do you already see where this is going? The problem begins when you stop obeying the contract of the interface you implement. I'll quote the javadoc for contains here:

  Returns true if this collection contains the specified element. More
  formally, returns true if and only if this collection contains at
  least one element e such that (o==null ? e==null : o.equals(e)).

This is the formal definition for contains that also applies to containsAll.

Now considering that a programmer seeing a Collection expects the contains method to work based on equals and countering with the fact that yours doesn't you're violating the "Principle Of Least Surprise".

This also leads to code handling collections behaving differently depending on what implementation you pass in, which is never desirable.

In general I think that relying solely on hashCode for contains is a bad idea, even when it's best practice to override hashCode so the resulting hashes of equal objects are equal and those of nonequal objects aren't.


Database of competition information I have the following (extremely simplified) database structure:

Table: Competitions

Id: string, unique

Table: Persons

Id: string, unique
Gender: string

Table: Results

CompetitionId: string, references Id on the Competitions table
PersonId: string, references Id on the Persons table
EventId: string
RoundId: string
Average: int

Any Competition may hold n Results. Each Result is assigned to one Person.

I would like to get all Results (filtered for Average > 0, EventId == "333", RoundId == "f", such that it is the one with the lowest Average within the other Results having the same CompetitionId. Furthermore, I only want to get the Results of which the according person is female (gender == "f").

I currently use a weird mixed LINQ construct that is both, ugly and inefficient. The query takes around 3 minutes on my machine (local MySQL database, Results row count is something close to 200k).

I know there are elegant and efficient ways to use one LINQ query, creating temporary tables, joining and such. I am not that much into it, thus I coded this ugly piece:

var femalePersonIds =
    from p in Persons
    where p.Gender == "f"
    select p.Id;

var results333 =
    from r in Results
    where (r.Average > 0) && (r.EventId == "333") && (r.RoundId == "f")
    orderby r.Average
    select r;

foreach (var c in Competitions) {
    var results =
        from r in results333
        where (r.CompetitionId == c.Id)
        select r;
    if (results.Count() > 0) {
        var bestCompResult = results.First();
        if (femalePersonIds.Contains(bestCompResult.PersonId)) {
            bestCompResult.Dump();
        }
    }
}

(This is LINQPad 4 compliant)

I would love to see any efficiency, elegance and shortening hints, in case you have some.

Try this one:

        var filteredResults =
        (from r in results
         where r.Average > 0 &&
               r.EventID == "333" &&
               r.RoundID == "f"
         select r).ToList();

        List<Results> bestResults = new List<Results>();
        var resultsInCompenitions = filteredResults.GroupBy(r => r.CompetitionID);
        foreach (var resultsInCompetition in resultsInCompenitions)
        {
            var bestResultInCompetition = resultsInCompetition.OrderBy(r => r.Average).FirstOrDefault();
            if (bestResultInCompetition != null)
            {
                bestResults.Add(bestResultInCompetition);
            }
        }

        var femaleBestResults =
            from r in bestResults
            join p in persons on r.PersonID equals p.ID
            where p.Gender == "f"
            select r;

       return femaleBestResults;


Pig Latin Translator in Python I am a relatively new Python programmer and made a simple Pig Latin to English translator and vice versa. I would just like it if someone reviewed my code and see if anything can be done to make it more Pythonic or efficient.

from time import sleep

def main():
    to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ")

    while to_piglatin not in("p", "e"):
        to_piglatin = input("[P/E]")

    to_piglatin = ("e", "p").index(to_piglatin.lower())

    if to_piglatin:
        lang = "pig latin"
    else:
        lang = "english"
    question = "Enter a sentence to translate it into {}: ".format(lang)
    sentence = input(question)

    if not sentence.isalpha():
        print("Invalid input, please do not use numbers or punctuation marks")
        sleep(1)
        main()

    split_sentence = sentence.split()

    if to_piglatin:
        translated_sentence = [translate_to_piglatin(word) for word in split_sentence]
    else:
        translated_sentence = [translate_to_english(word) for word in split_sentence]

    translated_sentence = " ".join(translated_sentence)
    print(translated_sentence)
    input()
    return True

def translate_to_piglatin(word):
    vowels = 'aeiou'
    list_word = list(word)
    list_word = [x.lower() for x in word]
    first_letter = list_word[0]
    last_letter = list_word[-1]

    if last_letter.isalpha() == False:
        list_word.remove(last_letter)
    if first_letter in vowels:
        list_word.append('y')
        list_word.append('a')
        list_word.append('y')
    else:
        list_word.remove(list_word[0])
        list_word.append(first_letter)
        list_word.append('a')
        list_word.append('y')

    translated_word = "".join(list_word)
    return translated_word

def translate_to_english(word):
    list_word = list(word)
    last_three_letters = list_word[-3:]
    last_three_letters = "".join(last_three_letters)

    if last_three_letters == 'yay':
        vowel = True
    else:
        vowel = False

    if vowel:
        for i in range(0,2):
            del list_word[-1]
    else:
        del list_word[-1]
        del list_word[-1]
        last_letter = list_word[-1]
        list_word.remove(last_letter)
        list_word.insert(0, last_letter)

    translated_word = "".join(list_word)
    return translated_word

main()

There is this thing in Python called unpacking.

The idea being that you can assign at once in several variables all the values contained in a single sequence:

>>> example = (1, 2, 3)
>>> a, b, c = example
>>> print(a, b, c)
1 2 3

In fact, this mechanism works with any iterable, including strings. What's even better is that this syntax got extended in Python 3 to support assigning the remaining of the items into a single variable:

>>> a, *b, c = range(5)
>>> print(a, c)
0 4
>>> print(b)
[1, 2, 3]

Now, let's make good use of it:

def translate_to_piglatin(word):
    vowels = 'aeiou'
    first_letter, *remaining_letters = word.lower()

    if first_letter in vowels:
        return word + 'yay'
    else:
        return ''.join(remaining_letters) + first_letter + 'ay'

Here I removed the need for first_letter, *inner_part, last_letter = word.lower() because I assumed that word was not ill-formed. Since you check for isalpha in your main, you don't really need to do so in this function.

An other improvement would be to define vowel as a constant outside of this function.

Converting back can also easily make use of unpacking:

def translate_to_english(word):
    if word.endswith('yay'):
        return word[:-3]
    else:
        *remaining_letters, last_letter, a, y = word
        return last_letter + ''.join(remaining_letters)

In this case:

I used a condition to enter the if rather than setting a boolean before;
I used the slice syntax to extract out the last 3 characters directly.

In both cases I returned from within the if to simplify the logic and the variable management. Each branch needing its own kind of data structure to work with.

The last thing unpacking can help you with is the choice of language/transformation function. If you define, for each input letter, the name of the language and the associated transformation function in a tuple, you can extract them out easily:

CONVERSION_INFOS = {
    'e': ('english', translate_to_english),
    'p': ('pig latin', translate_to_piglatin),
}

def main():
    to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ").lower()

    while to_piglatin not in CONVERSION_INFOS:
        to_piglatin = input("[P/E]").lower()

    lang, translate = CONVERSION_INFOS[to_piglatin]

    sentence = input("Enter a sentence to translate it into {}: ".format(lang))

    if not sentence.isalpha():
        print("Invalid input, please do not use numbers or punctuation marks")
        sleep(1)
        main()

    translated_sentence = ' '.join(translate(word) for word in sentence.split())

    print(translated_sentence)
    input()
    return True

Note the use of lower on the first two inputs so your while loop can break if the user input 'E' or 'P'.

I also merged some lines together to remove the intermediate variables you only use once and reduce the burden of understanding your code for the reader.

Lastly, you can improve the workflow of your program by making a correct use of return values. You don't need to return something at the end of a function in Python (meaning return True in main is unneccessary) when there is no associated meaning. However, we can make use of it:

We return True if we correctly translated a sentence;
We return False if we couldn't.

def main():
    to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ").lower()

    while to_piglatin not in CONVERSION_INFOS:
        to_piglatin = input("[P/E]").lower()

    lang, translate = CONVERSION_INFOS[to_piglatin]

    sentence = input("Enter a sentence to translate it into {}: ".format(lang))

    if not sentence.isalpha():
        print("Invalid input, please do not use numbers or punctuation marks")
        return False

    translated_sentence = ' '.join(translate(word) for word in sentence.split())

    print(translated_sentence)
    return True

This let you handle retries outside of the function:

while not main():
    pass  # or time.sleep(1) like you did, but it feels a bit unresponsive

You may also want to change the name of the function because it doesn't convey much meaning when calling it like that.


Eloquent JavaScript chessboard Is this a good way to solve the quiz "Chessboard" from http://eloquentjavascript.net/02_program_structure.html ?

  Write a program that creates a string that represents an 8×8 grid, using newline characters to separate lines. At each position of the grid there is either a space or a “#” character. The characters should form a chess board.

  When you have a program that generates this pattern, define a variable size = 8 and change the program so that it works for any size, outputting a grid of the given width and height.

This is my code:

 size = 10;

    grid = ""

    for (var i = 1; i <= size; i++) {
      for (var j = 1; j <= size; j++) {
        if (i % 2 === 0) {
         grid+= "# "
        } else {
         grid+= " #"
        }
      }
      grid+= "\n"
    }

    console.log(grid)

Fun question;

you should write a function that takes a parameter instead of just writing the code
A chessboard has lots of repetition, take a minute to ponder how String.repeat could make this code much simpler.
Your indentation is not perfect, consider using a site like http://jsbeautifier.org/
I am not a big fan of var within the loop, I would declare var up front.

This is a possible solution that provides the right size of the board:

function createChessboardString(size){

  const line = ' #'.repeat( size ),
        even = line.substring(0,size),
        odd = line.substring(1,size+1);
  let   out = '';

  while(size--){
    out = out + ((size % 2) ? odd: even ) + '\n';
  }

 return out;
}

console.log(createChessboardString(8));

You could consider for very large boards that the board in essence repeats
odd + '\n' + even, so you could repeat that as well. The problem for me is that there are too many corner cases to consider. So personally I would go for the above for any board size < 1000.


Checking for user permissions The idea here is we have Roles, Permissions, and a table called PermissionRoles that connect the two. So a Permissions can be in many Roles, and many Permissions can have the same Role. So what the following code does is:

Get all the permissions required for a user to authenticate, by
ActionName
Loop through each of these permissions
Get the list PermissionsRole records that have one of those Permission objects
Get the list of Roles from the list of PermissionRoles
Loop through that list of Roles
Check if a user is in any of those roles

This is the code, I feel like there may be a more efficient way to write this. It is implied that I have instantiated a UserManager and Database Context.

string[] permissions = Permissions.Split(',').ToArray();

IEnumerable<string> perms = permissions.Intersect(db.Permissions.Select(p => p.ActionName));
List<IdentityRole> roles = new List<IdentityRole>();

if (perms.Count() > 0)
{
    foreach (var item in perms)
    {
        var currentUserId = httpContext.User.Identity.GetUserId();

        var permissionsRoles = db.PermissionRoles.Where(p => p.Permission.ActionName == item && p.CompanyId == companyId).ToList();
        var existingRoles = dbu.Roles.Select(x => x.Id).Intersect(permissionsRoles.Select(x => x.RoleId)).ToList();
        foreach (var role in existingRoles)
        {
            ApplicationRole thisRole = dbu.Roles.Find(role);
            if (userManager.IsInRole(currentUserId, thisRole.Name))
            {
                return true;
            }
        }
    }
}
return false;

I would say you do too many things at once. The important thing here is this check:

if (userManager.IsInRole(currentUserId, thisRole.Name))
{
    return true;
}

This needs a list of role-names to iterate over:

foreach (var roleName in roleNames)
{
    if (userManager.IsInRole(userId, roleName))
    {
        return true;
    }
}

return false;

The list of role-names seems to be dependent of some given permissions and a company:

var givenPermissions = GetGivenPermissions();
var roleNames = GetRoleNames(companyId, givenPermissions);

The first is easy enough to extract from the given code:

IEnumerable<string> GetGivenPermissions()
{
    return Permissions.Split(',').ToArray();
}

The other one is trickier, but I think I got it right:

IEnumerable<string> GetRoleNames(string companyId, IEnumerable<string> givenPermissions)
{
    var existingPermissions = db.Permissions.Select(p => p.ActionName);
    var relevantPermissions = givenPermissions.Intersect(existingPermissions);

    foreach (var permission in relevantPermissions)
    {
        var allRoles = db.Roles.Select(x => x.Id);
        var relevantPermissionRoles = db.PermissionRoles
                                        .Where(p => p.Permission.ActionName == permission && p.CompanyId == companyId)
                                        .Select(x => x.RoleId)
                                        .ToList();

        var relevantRoles = allRoles.Intersect(relevantPermissionRoles).ToList();

        foreach (var role in relevantRoles)
        {
            var thisRole = db.Roles.Find(role);
            var name = thisRole.Name;
            yield return name;
        }
    }
}

This is one nasty query:

It's a good mix of things querying the database and things filtered in memory. - It's hard to figure out what it actually do, and what the requirements for the roles really are.
It's also a mix of code operating on objects and primitive values, which is probably why db.Roles is queried twice.

What I would like to see in here is something that:

First retrieve necessary data from the database, once.
Filter it against in-memory data.
Return the resulting role names.


Simple function that execute an instruction every X time until Y condition is accomplished What do you think about this implementation?

inline fun every(duration: Long, timeUnit: TimeUnit, whileCondition: () -> Boolean = { true }, function: () -> Unit) {
    while (whileCondition()) {
        function()
        Thread.sleep(timeUnit.toMillis(duration))
    }
}

This allows calling the following statement:

every(5, TimeUnit.SECONDS){
    println("Hello world")
}

Usually it's a bad idea to put to sleep whole thread. I'd recommend use Java Timer or kotlin's wrapper for it.

Here is an example:

fixedRateTimer("default", false, 0L, 1000){
    println("Hello!")
    if (theEndIsNear) cancel()
}


Dynamic Fibonacci algorithm in x86 (MASM) My task was to implement this algorithm that uses memoization to calculate fibonnaci numbers:

int fib(int n)
{
  int f[n+2];   // 1 extra to handle case, n = 0
  int i;

  /* 0th and 1st number of the series are 0 and 1*/
  f[0] = 0;
  f[1] = 1;

  for (i = 2; i <= n; i++)
  {
      f[i] = f[i-1] + f[i-2];
  }
  return f[n];
}

This proved to be more challenging because it involves local variables. What's more, it's a local array with a variable initial size.

;*******************************************************************************
; This program calculates fibonacci numbers with a dynamic programming
; algorithm.
;
; Author: William

                .386
                .model  flat, stdcall
                .stack  4096

ExitProcess     PROTO,  dwExitCode:DWORD

.code

;-------------------------------------------------------------------------------
; FibAsm( int n )
;
; Calculates fibonacci numbers using memoizaiton
;
; Entry: n      @ ebp + 8                       ; DWORD unsigned int up to 47
;
; Local: f[]                                    ; array to store fibs
;
; Exit: the fib number is returned in eax
;-------------------------------------------------------------------------------
FibAsm:
                ; set up stack frame
                push    ebp
                mov     ebp, esp
                push    ebx
                push    ecx
                push    esi
                push    edi

                mov     ebx, [ebp + 8]          ; n stored in ebx

                ; new f[] of size n + 2
                lea     edi, [ebx + 2]          ; save size of array in edi
                shl     edi, 2                  ; mul by 4, size of DWORD int
                sub     esp, edi                ; allocate space for f[]
                                                ; pointer to f[] in esp !

                ; f[0] = 0 and f[1] = 1
                mov     DWORD PTR[esp], 0
                mov     DWORD PTR[esp + 1 * 4], 1

                ; for (i = 2; i <=n; i++)
                mov     ecx, 2                  ;loop count i set to 2
L1:             cmp     ecx, ebx
                jg      endL1

                ; f[i] = f[i - 1] + f[i - 2]
                lea     eax, [ecx - 1]
                mov     esi, DWORD PTR[esp + eax * 4]
                dec     eax
                add     esi, DWORD PTR[esp + eax * 4]
                mov     [esp + ecx * 4], esi

                inc     ecx                     ;inc i
                jmp     L1
endL1:
                ; return f[n]
                mov     eax, DWORD PTR[esp + ebx * 4]

                ;clean up stack, restore regs
                add     esp, edi                ; add array size to esp
                pop     edi
                pop     esi
                pop     ecx
                pop     ebx
                pop     ebp
                ret     4

main:
                push    47
                call    FibAsm
                INVOKE  ExitProcess, 0

                END             main

What I ended up doing after a long time playing with the stack frame was to calculate the size first and save that in a register in order to clean up the stack later. Also I used the esp register as the start of the array instead of an offset from ebp. That seems counter intuitive, but it was simpler than calculating some distance from ebp (which would have to account for all the registers than were pushed to the stack after ebp).

Anyway, I'm wondering how this is normally done and looking for any other criticism as well.

I'm wondering how this is normally done

Yours is as good a solution as anyone else's.
That said, a solution that pushes the elements would not need to address these elements relative to EBP at all.

; new f[] of size n + 2
lea     edi, [ebx + 2]             ; save size of array in edi
shl     edi, 2                     ; mul by 4, size of DWORD int

You can calculate the required stackspace in a single LEA instruction:

; new f[] of size n + 2
lea     edi, [ebx * 4 + 8]

; f[0] = 0 and f[1] = 1
mov     DWORD PTR[esp], 0
mov     DWORD PTR[esp + 1 * 4], 1
; for (i = 2; i <=n; i++)
mov     ecx, 2                     ; loop count i set to 2

All of these assignments require a 32-bit immediate value. That's a lot of bytes! Because later we need ECX=2, we can start with clearing ECX and incrementing twice towards 2, storing the intermediate results in memory:

; f[0] = 0 and f[1] = 1
xor     ecx, ecx
mov     [esp + 0 * 4], ecx         ; f[0] = 0
inc     ecx
mov     [esp + 1 * 4], ecx         ; f[1] = 1
inc     ecx                        ; loop count i set to 2

But wait, there still room for improvement!

The address [esp + 0 * 4] requires a modR/M-byte, sib-byte and an 8-bit displacement.
The address [esp + ecx * 4] requires a modR/M-byte and a sib-byte.

Final draft:

; f[0] = 0 and f[1] = 1
xor     ecx, ecx
mov     [esp + ecx * 4], ecx       ; f[0] = 0
inc     ecx
mov     [esp + ecx * 4], ecx       ; f[1] = 1
inc     ecx                        ; loop count i set to 2

; f[i] = f[i - 1] + f[i - 2]
lea     eax, [ecx - 1]
mov     esi, DWORD PTR[esp + eax * 4]
dec     eax
add     esi, DWORD PTR[esp + eax * 4]
mov     [esp + ecx * 4], esi

You can easily address the 2 previous array elements by adding offsets -4 and -8.
This liberates the ESI register that also no longer needs to be preserved.

; f[i] = f[i - 1] + f[i - 2]
mov     eax, [esp + ecx * 4 - 8]   ; f[i - 2]
add     eax, [esp + ecx * 4 - 4]   ; f[i - 1]
mov     [esp + ecx * 4], eax       ; f[i]

        mov     ecx, 2
L1:     cmp     ecx, ebx
        jg      endL1
        ...
        inc     ecx        ; inc i
        jmp     L1
endL1:

A loop like this is wasteful because of the uncondional jump that gets executed on every iteration, on top of the conditional jump that needs to be  processed anyway. You can make the conditional jump the only jump that gets executed repeatedly.

        mov     ecx, 2
        jmps    L2         ; once
L1:     ...
        inc     ecx        ; inc i
L2:     cmp     ecx, ebx
        jbe     L1         ; repeatedly


Parsing function is 50 lines long This is a parsing function that will at tildes (~) to end of search terms in certain circumstances.

Example an inputs and outputs:

Input:                        Output:
name:(john doe)               name:(john~ doe~)
name:[andy TO charlie]        name:[andy TO charlie]
john doe                      john~ doe~
james NOT jane                james~ NOT jane
james NOT (james smith)       james~ NOT (james smith)
james NOT jane smith          james~ NOT jane smith~
name:"john doe" australia     name:"john doe" australia~

function addTilde(string) {
    if (!/[\[\[\]~"(NOT)\-\!\d\(\)(OR)(AND)\&\|\: ]/.test(string)) {
        string = string.concat("~");
    }

    return string;
};

function fuzzQuery(rawQuery) {
    /*split the string into spaces, brackets, double quotes and words*/
    re = /(?=[()\[\] "])|(?=[^\W])\b/;
    strSplit = rawQuery.split(re);

    newQuery = "";

    for (var i = 0; i < strSplit.length; i++) {
        var s = strSplit[i];
        var newElement = "";

        /*if it contains a [ or "*/
        if (s.indexOf("\x22") != -1 || s.indexOf("[") != -1) {

            /*determine closing symbol*/
            var closingSymbol;
            if (s == "\x22") {
                closingSymbol = "\x22";
                newElement = newElement.concat(strSplit[i++]); /*need to skip opening one for double quotes*/
            } else closingSymbol = "]";

            /*concat elements together until closing element found)*/
            do {
                newElement = newElement.concat(strSplit[i]);
            }
            while (strSplit[i++] != closingSymbol)
        }

        /*if it contains a NOT*/
        else if (s.indexOf("NOT") != -1) {
            newElement = strSplit[i++]; /*concat the NOT*/

            /*concat any spaces*/
            while (strSplit[i] == " ") {
                newElement = newElement.concat(strSplit[i++]);

            }

            if (strSplit[i] == "(") {
                do {
                    newElement = newElement.concat(strSplit[i]);
                }
                while (strSplit[i++] != ")")

            } else newElement = newElement.concat(strSplit[i++]);

        } else(newElement = strSplit[i]);

        newElement = addTilde(newElement);
        newQuery = newQuery.concat(newElement);
    }

    return newQuery;
};

Now fuzzQuery is quite a long method.
It essentially has five parts.

Split the initial query out into elements.
Loop through each element.

a) Concat square brackets and double quotes.

else b) concat NOTs.

now add tilde to element if appropriate
Return the join the elements back together and return the new query.

What I was thinking is that you could pass off steps two and three to their own methods, so that the whole query looks something like (but not exactly like!):

function fuzzQuery(rawQuery)
{
    strSplit = splitQuery(rawQuery);
    concatSqrAndDblQuotes(strSplit);
    concatNots(strSplit);
    return putBackTogether(strSplit);
}

ie.

function doSquareAndDblQuotes(strSplit, i) {
    if (s.indexOf("\x22") != -1 || s.indexOf("[") != -1) {

        /*determine closing symbol*/
        var closingSymbol;
        if (s == "\x22") {
            closingSymbol = "\x22";
            newElement = newElement.concat(strSplit[i++]); /*need to skip opening one for double quotes*/
        } else closingSymbol = "]";

        /*concat elements together until closing element found)*/
        do {
            newElement = newElement.concat(strSplit[i]);
        }
        while (strSplit[i++] != closingSymbol)
    }

    return newElement;
}

But the problem is here that we'd need to be keeping track of a few variables being changed in this function. ie. the i counter, and whether or not that if statement was executed. So you could start using globals (is that even a thing in javascript?)... and it gets messy.

So possibly another way, would be to create an object that you pass in, and return, which keeps track of these variables.

What do you think?

You aren't using regular expressions to your advantage.  Capture, don't split.  Capturing helps you analyze the tokens you are interested in.  Splitting just gets you the location of the delimiters.

function fuzzQuery(rawQuery) {
    "use strict";
    //              ( 1 )     (   2   )    (   3   ) (    4    ) (     5    ) (  6   )
    var re = /\s*(?:(NOT)\s+)?([a-z]+:)?(?:("[^"]*")|(\([^)]*\))|(\[[^\]]*\])|([a-z]+))\s*/g;
    var matches;
    var lastIndex = -1;
    while (matches = re.exec(rawQuery)) {
        var relOp      = matches[1],
            qualifier  = matches[2],
            quotedStr  = matches[3],
            parensStr  = matches[4],
            bracketStr = matches[5],
            bareWord   = matches[6];
        lastIndex = re.lastIndex;

        console.log("relOp=" + relOp +
                    ", qualifier=" + qualifier +
                    ", quotedStr=" + quotedStr +
                    ", parensStr=" + parensStr +
                    ", bracketStr=" + bracketStr +
                    ", bareWord=" + bareWord);
    }
    if (lastIndex != rawQuery.length) {
        console.log("Junk=" + rawQuery.substring(lastIndex));
    }
}

Examples:

name:(john doe)

relOp=undefined, qualifier=name:, quotedStr=undefined, parensStr=(john doe), bracketStr=undefined, bareWord=undefined

name:[andy TO charlie]

relOp=undefined, qualifier=name:, quotedStr=undefined, parensStr=undefined, bracketStr=[andy TO charlie], bareWord=undefined

john doe

relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=john
relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=doe

james NOT jane

relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=james
relOp=NOT, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=jane

james NOT (james smith)

relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=james
relOp=NOT, qualifier=undefined, quotedStr=undefined, parensStr=(james smith), bracketStr=undefined, bareWord=undefined

james NOT jane smith

relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=james
relOp=NOT, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=jane
relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=smith

name:"john doe" australia

relOp=undefined, qualifier=undefined, quotedStr="john doe", parensStr=undefined, bracketStr=undefined, bareWord=undefined
relOp=undefined, qualifier=undefined, quotedStr=undefined, parensStr=undefined, bracketStr=undefined, bareWord=australia


A Java class for calculating size of iterable/array v2 Version 2 of A Java class for calculating size of iterable/array

import java.util.Collection;

/**
 * Created by IDEA on 16/11/14.
 */
public class Size {
    public static int size(Iterable<?> data) {
        if (data instanceof Collection) {
            return ((Collection<?>) data).size();
        }
        int counter = 0;
        for (Object i : data) {
            counter++;
        }
        return counter;
    }

    public static int size(int[] data) {
        return data.length;
    }

    public static int size(double[] data) {
        return data.length;
    }

    public static int size(float[] data) {
        return data.length;
    }

    public static int size(short[] data) {
        return data.length;
    }

    public static int size(boolean[] data) {
        return data.length;
    }

    public static int size(char[] data) {
        return data.length;
    }

    public static <T> int size(T[] data) {
        return data.length;
    }
}

if (data instanceof Collection) {
    return ((Collection<?>) data).size();
}

This looks like it was meant to improve performance for big collections. But, that instanceof will prevent certain compiler optimizations and you will get huge performance hit for smaller collections. It's likely not worth it, as most collections are small. This could be avoided by having separate method for Collections and Iterables.

Universal utilities like this improve neither readability nor performance. I do not see how Size.size(x) is more readable than x.length or x.size(). The only benefit is that you don't have to think which one to use.

I would leave only method for calculating the size of Iterables and use size()/length for collections/arrays.

/**
 * Created by IDEA on 16/11/14.
 */

Such comments aren't really useful.

public class Size {

I would make the class final and the constructor private as there is no reason to extend or instantiate class with only static methods.


Sorting an array using a Binary Search Tree with C++ Given an array of integer (all different from each other), this program creates a BST with the array elements in order to sort them and put them back into the array, using the BST properties.

#include<iostream>
using namespace std;

struct Node
{
    int label;
    Node* left;
    Node* right;

    ~Node() {
      delete left;
      delete right;
      }
};

void insertSearchNode(Node* &tree, int x)       //insert integer x into the BST
{
    if(!tree){
        tree = new Node;
        tree->label = x;
        tree->right = NULL;
        tree->left = NULL;
        return;
    }
    if(x < tree->label) insertSearchNode(tree->left, x);
    if(x > tree->label) insertSearchNode(tree->right, x);
    return;
}

void insertArrayTree(int arr[], int n, Node* &tree)     //insert the array integer into the nodes label of BST
{
    for(int i=0; i<n; i++){
        insertSearchNode(tree, arr[i]);
    }
    return;
}

int insertIntoArray(int arr[], Node* &tree, int i)      //insert into the array the node label touched during an inorder tree traversal
{
    if(!tree) return i;
    i = insertIntoArray(arr, tree->left, i);
    arr[i] = tree->label;
    i++;
    i = insertIntoArray(arr, tree->right, i);
    return i;

}

int main()
{
    Node* maintree;
    maintree = NULL;
    int num = 7;
    int arr[num] = {120, 30, 115, 40, 50, 100, 70};
    insertArrayTree(arr, num, maintree);      //insert elements into BST
    insertIntoArray(arr, maintree, 0);        //modify array sorting his elements using the BST

    for(int y=0; y<num; y++) cout<< arr[y] << ' ';

    return 0;
}

Output:

30 40 50 70 100 115 120

Any advice is welcome.

Your code is readable and easy to understand. Said that I would like to make you think about how you handle the operations in your BSTree.

You are building the functions alone, and what that causes is to manually manage these functions and compromise the structure of your tree if accidentally you write something wrong.

You should opt for an Internal management rather than external one. For example:

struct BinaryNode
{
    int label_;
    Node* left_;
    Node* right_;

    //having constructors makes you write less code

    BinaryNode(Node *left, const int &label, Node *right) :
        label_(label), left_(left), right_(right)
    {
    }

    //const [datatype]&
    //means to not copy the value of the parameter
    BinaryNode(const int &label) :
    //nullptr is typesafe use it instead of NULL which is a macro
        label_(label), left_(nullptr), right_(nullptr)
    {
    }

    ~BinaryNode() {
        delete left_;
        delete right_;
    }
};

class BinarySearchTree
{
private:
    BinaryNode *root_;

protected://if some derived class is needed (as an AVLTree)
    //Google about virtual functions for abstract classes (pure virtual too)
    virtual void Insert(const int &x, Node *node)
    {
        if (x < node->label_)
            if (node->left_ == nullptr)
                node->left_ = new BinaryNode(t);
            else Insert(t, node->left_);
        else if (x > node->label_)
            if (node->right_ == nullptr)
                node->right_ = new BinaryNode(t);

        //If x is in the tree it does not include x again
    }

public:
    virtual void Insert(const int &x)
    {
        Insert(x, this->root_);
    }
};

Don't misunderstand me, Your code is useful and is pretty (is something good to read). But, manage things externally can be messy.

Also, I recommend you to read the Google C++ Code Style

Hope it helped.

Edit: What does const int& x means?

const int& x is a constant reference to a value of type int so, when you use it, your function does not copy the sent value, instead it "sees where the value is allocated" takes it in account and then starts the procedure you describe within the function, additionally you cannot modify that value (because it's constant [const])

the main difference is with const [datatype]& you save space. With [datatype] [paramName] you Copy the value which implies allocate the copied value.

If you feel lost in some part read it slowly it has many concepts, and remember practice makes this make sense.


How can this Encryption Class be more secure? How secure is this encryption?
Secondly, is there something I could do to "turn it up a notch" and make it 'more' secure?

Imports System.Security.Cryptography

Public Class clsCrypt
Private TripleDES As New TripleDESCryptoServiceProvider

'create and return the key and initialization vector hash
Private Function TruncateHash(ByVal key As String, ByVal length As Integer) As Byte()

    Dim sha1 As New SHA1CryptoServiceProvider

    ' Hash the key.
    Dim keyBytes() As Byte = System.Text.Encoding.Unicode.GetBytes(key)
    Dim hash() As Byte = sha1.ComputeHash(keyBytes)

    ' Truncate or pad the hash.
    ReDim Preserve hash(length - 1)
    Return hash

End Function

'initialize the class
Sub New(ByVal key As String)

    ' Initialize the crypto provider.
    TripleDES.Key = TruncateHash(key, TripleDES.KeySize \ 8)
    TripleDES.IV = TruncateHash("", TripleDES.BlockSize \ 8)

End Sub

'encrypt the passed text
Public Function EncryptData(ByVal plaintext As String) As String

    ' Convert the plaintext string to a byte array.
    Dim plaintextBytes() As Byte = System.Text.Encoding.Unicode.GetBytes(plaintext)

    ' Create the stream.
    Dim ms As New System.IO.MemoryStream

    ' Create the encoder to write to the stream.
    Dim encStream As New CryptoStream(ms, TripleDES.CreateEncryptor(),
                                      System.Security.Cryptography.CryptoStreamMode.Write)

    ' Use the crypto stream to write the byte array to the stream.
    encStream.Write(plaintextBytes, 0, plaintextBytes.Length)
    encStream.FlushFinalBlock()

    ' Convert the encrypted stream to a printable string.
    Return Convert.ToBase64String(ms.ToArray)

End Function

'decrypt passed text
Public Function DecryptData(ByVal encryptedtext As String) As String

    ' Convert the encrypted text string to a byte array.
    Dim encryptedBytes() As Byte = Convert.FromBase64String(encryptedtext)

    ' Create the stream.
    Dim ms As New System.IO.MemoryStream

    ' Create the decoder to write to the stream.
    Dim decStream As New CryptoStream(ms, TripleDES.CreateDecryptor(),
                                      System.Security.Cryptography.CryptoStreamMode.Write)

    ' Use the crypto stream to write the byte array to the stream.
    decStream.Write(encryptedBytes, 0, encryptedBytes.Length)
    decStream.FlushFinalBlock()

    ' Convert the plaintext stream to a string.
    Return System.Text.Encoding.Unicode.GetString(ms.ToArray)

End Function
End Class

Lastly, would this encryption class function the same if I passed in a different key in different Subs? Example:

Private Sub EmployeeFrm_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load

    Dim strKey As String = "Key1" 'encryption Key
    Dim clsEncrypt As clsCrypt         'Assigns a variable to clsCrypt class
    clsEncrypt = New clsCrypt(strKey)   ' creates a new isntance of the clsCrypt class

And also:

Private Sub InsertBtn_Click(sender As System.Object, e As System.EventArgs) Handles InsertBtn.Click

    Dim strKey As String = "Strong3rKeY!" 'encryption Key
    Dim clsEncrypt As clsCrypt         'Assigns a variable to clsCrypt class
    clsEncrypt = New clsCrypt(strKey)   ' creates a new isntance of the clsCrypt class

Both are in the same form, just during different events.

Is this even a good way to utilize encryption? I am using it to encrypt specific strings and insert them into the database.

Some issues:

No MAC => say hello to padding oracles. This is fatal flaw that allows decryption of the message in many usage scenarios (when there is an active attacker).
You use CBC mode with constant IV. A new IV must be chosen for each message. It should be unpredictable and random.
Your keying is only secure with really strong passwords (over 100 bits of entropy). I recommend using an actual binary key drawn from a secure CSPRNG encoded with Base64 instead of a password.

A related stylistic problem is that you don't clearly distinguish between keys (which should be uniformly random) and passwords.
Does the key ever leave the trusted systems? It looks like you're hardcoding a key into a client, which might run on an untrusted machine. Even if it stays on the server, put it in a config file, not the code.

Your program should be secure even if the whole code is known. Logistically it's also much easier to keep a config file secure, since it doesn't need to be checked into version control.
Why would you choose 3DES over AES in new code? AES is faster and stronger. It also has 128 bit blocks instead of 64 bit blocks, so it doesn't weaken once you encrypt a couple of gigabytes using the same key.
All those streams are unnecessarily complicated. Simply call encryptor.TransformFinalBlock on a byte array instead.


Smart way to shorten the current working directory in pure Bash I'm trying to create a shell prompt that displays the $PWD in a "smart" way. I want to display the top two directories rather than the full path. If $HOME is the parent (or parent of the parent) directory, I want to show it like ~.

Assuming my $HOME is "/home/user", here are a few test cases with expected output according to several $PWD values:

/                  -> /
/home              -> /home
/home/user         -> ~
/home/user/foo     -> ~/foo
/home/user/foo/bar -> foo/bar
/usr/local         -> /usr/local
/usr/local/games   -> local/games
/home/userr        -> /home/userr

Currently, my solution is as follow:

path=$PWD

# $n is the number of "/" in the path indicating that there is more than 2 dirs
# In such case, the path should be truncated
# If the path contains root / (like "/home/usr"), 3 "/" implies 3 dirs
# If the path is "~/foo" then only 2 "/" are required
# If ${PWD} starts with ${HOME}, we can replace this part with "~"
# A slash need to be appended to "${HOME}" to avoid false positive
n=3
if [[ ${path}/ == ${HOME}/* ]]; then
  path=~${path:${#HOME}}
  n=2
fi

# Count the number of "/" in the path
# If there is more than $n "/", this means the path contains more than 2 dirs
# In this situation, we need to extract the two last dirs only from the path
slashes=${path//[^\/]}
if [[ ${#slashes} -ge $n ]]; then
  dir=${path##*/}
  parentpath=${path%/*}
  parentdir=${parentpath##*/}
  path=$parentdir/$dir
fi

echo $path

This function is intended to be used in my bash shell prompt. As this will be issued each time I type a command in my terminal, and as I will keep this in my .bashrc for probably several years to come, I'm looking to "optimize" this function.

Could the solution have been implemented otherwise, or could this function be improved in some way in order to minimize it's overall "footprint"?

Returning values by variable is 3-10 times faster than capturing an echo with $( ).  Your temp variable is already global.  Just name it something similar to what the surrounding function is named.

Whitespace and comments affect bash performance negatively.  It's not a big penalty, just something to be aware of.

A regular expression can do the "last two path components" extraction very compactly.

cwd() {
  cwd=$PWD
  [[ $cwd/ == $HOME/* ]] && cwd=~${cwd#$HOME}
  [[ $cwd =~ ./([^/]+/[^/]+)$ ]] && cwd=${BASH_REMATCH[1]}
}

PROMPT_COMMAND=cwd

PS1="\$cwd \$"

I'd say not to count the tilde as a path component: it's only one extra character that carries a lot of information, well worth the single column.   The regex is then:

  [[ $cwd =~ (..|[^~])/([^/]+/[^/]+)$ ]] && cwd=${BASH_REMATCH[2]}


Credit card validator in Python 3.7.1 I am doing some projects so I can teach myself programming. The one that I finished states:
"Credit Card Validator - Takes in a credit card number from a common credit card vendor  and validates it to make sure that it is a valid number (look into how credit cards use a checksum)."
Would be glad if i can get a constructive feedback on this one.

import sys

cardnumb_input = input('Enter your credit card number: ')

def list_tweak():
    try:
        cardnumb = [int(i) for i in cardnumb_input]
        cardnumb[0::2] = [x * 2 for x in cardnumb[0::2]]
        cardnumb = [str(i) for i in cardnumb]
        cardnumb = list(''.join(cardnumb))
        cardnumb = [int(i) for i in cardnumb]
        return cardnumb

    except ValueError:
        print('Please enter only number in format: xxxxxxxxxxxxxxxx')
        sys.exit(1)

def valid_or_not():
    total = sum(list_tweak())
    if total % 10 != 0:
        print('It seems that this credit card number is not valid.')
    else:
        print('This credit card number is valid. ')

valid_or_not()

What should the type signature of these functions be?

I would really separate the IO from data. In addition, I would display data that you already have. I would not have valid_or_not print stuff out. Instead I would have it return a value. But what value?

Furthermore, don't read global variables. valid_or_not and list_tweak should take paramters.

We'll get to printing stuff in the end.

What do you want to do with an invalid credit card number?

You are both throwing an error, and also printing out whether it is a valid card number. I would say there is a couple ways to go, but I prefer this approach:

  Return an enum that ends up being either: VALID, INVALID_CHECKSUM, or INVALID_FORMAT. Instead of doing printing.

Rework the flow of valid_or_not.

Consider reworking valid_or_not to do the following:

Check to see if the format is valid, and return INVALID_FORMAT if it is not.
The run list_tweak, but don't do try-except on it, return either INVALID_CHECKSUM or VALID depending on list_tweak.

Naming.

I am not sure what list_tweak means, but I am assuming only because I've seen this problem show up time and time again that it is the Luhn algorithm. Name it something appropriate like luhn_checksum.

IO

So what should we really be doing for input? First we should have a main function (see the link for more information why. Thus we get:

def main():
    cardnumber = input('Enter your credit card number: ')
    print(valid_or_not(cardnumber))

if __name__ == '__main__':
    main()

See all the data is manipulated in valid_or_not which is then displayed, instead of displaying information withing valid_or_not).


Calculate EMI based on principal, rate, duration (month or years) I am trying to create an EMI calculator for a loan. The problem description is:

Given a principal amount, rate of interest, monthly or quarterly installment terms and duration of loan in months/years and print EMI amount.
Input format:Principal, Rate of Interest per year, M/Q for Monthly or quarterly installment, duration of year in Year (Y) or Month (M)
Example Inputs:
10000, 12, M, 2Y: Principal = 10000, Rate of Interest = 12%, Monthly installment, Duration = 2 years
50000, 10.5, Q, 36M: Principal = 50000, Rate of Interest = 10.5%, Quarterly installment, Duration = 36 months

What are the flaws in this code? How can I perfect it?
def EMI_calculator(P,R , M,d):
#set rate of interest monthly
    print type(R)
    r= (R/(12.0 *100))
    print r
#chechk if monthly or quaterly installment
    if M== 'Q':
        x=4
    else:
        x=1

    if d[-1] =='Y':
        print "duration in years"
        n = d[0:(len(d)-1)]
        print n
        m = 12 *int(n)
        print m
    elif d[-1] =='M':
        print "duration is in months"
        n = d[0:(len(d)-1)]
        print n
        m = 1 *int(n)
        print m

    EMI = x*(P*r*(1+r)**m)//((1+r)**m - 1)
    return EMI

print EMI_calculator(10000,12,'Q','3Y')

In the comments to your other questions some things were mentioned that are also applicable to this question as well, like:

Try to follow PEP 8 style guide.
Single letter names should be avoided.
Avoid useless comments, make your code self explanatory.
Check for invalid input, raise exceptions.

Now, you have some magic numbers like 12, 100 and 4. Give them names. Make them constants. Don't leave them like that.

Some prints don't make a lot of sense, like printing type of R or printing if duration is in years or in months. That looks more like information for debugging. I suggest you to take a look at logging module and give it a try. But for such a small program it is probably an overkill.

This n = d[0:(len(d)-1)] is repeated twice in your code and it can be taken out of that if-elif statement. Moreover it can be simplified as n = d[:-1].

Instead of writing

EMI = x*(P*r*(1+r)**m)//((1+r)**m - 1)
return EMI

you can write just

return x*(P*r*(1+r)**m)//((1+r)**m - 1)

Finally, wrap your print EMI_calculator(10000,12,'Q','3Y') like this:

if __name__ == '__main__':
    print EMI_calculator(10000, 12, 'Q', '3Y')

You can read about it here.


Creating a csv file using scrapy I've created a script using Python in association with Scrapy to parse the movie names and its years spread across multiple pages from a torrent site. My goal here is to write the parsed data in a CSV file other than using the built-in command provided by Scrapy, because when I do this:

scrapy crawl  torrentdata -o outputfile.csv -t csv

I get a blank line in every alternate row in the CSV file.

However, I thought to go in a slightly different way to achieve the same thing. Now, I get a data-laden CSV file in the right format when I run the following script. Most importantly I made use of a with statement while creating a CSV file so that when the writing is done the file gets automatically closed. I used crawlerprocess to execute the script from within an IDE.

  My question: Isn't it a better idea for me to follow the way I tried below?

This is the working script:

import scrapy
from scrapy.crawler import CrawlerProcess
import csv

class TorrentSpider(scrapy.Spider):
    name = "torrentdata"
    start_urls = ["https://yts.am/browse-movies?page={}".format(page) for page in range(2,20)] #get something within list
    itemlist = []

    def parse(self, response):
        for record in response.css('.browse-movie-bottom'):
            items = {}
            items["Name"] = record.css('.browse-movie-title::text').extract_first(default='')
            items["Year"] = record.css('.browse-movie-year::text').extract_first(default='')
            self.itemlist.append(items)

        with open("outputfile.csv","w", newline="") as f:
            writer = csv.DictWriter(f,['Name','Year'])
            writer.writeheader()
            for data in self.itemlist:
                writer.writerow(data)

c = CrawlerProcess({
    'USER_AGENT': 'Mozilla/5.0',
})
c.crawl(TorrentSpider)
c.start()

By putting the CSV exporting logic into the spider itself, you are re-inventing the wheel and not using all the advantages of Scrapy and its components and, also, making the crawling slower as you are writing to disk in the crawling stage every time the callback is triggered.

As you mentioned, the CSV exporter is built-in, you just need to yield/return items from the parse() callback:

import scrapy

class TorrentSpider(scrapy.Spider):
    name = "torrentdata"
    start_urls = ["https://yts.am/browse-movies?page={}".format(page) for page in range(2,20)] #get something within list

    def parse(self, response):
        for record in response.css('.browse-movie-bottom'):
            yield {
                "Name": record.css('.browse-movie-title::text').extract_first(default=''),
                "Year": record.css('.browse-movie-year::text').extract_first(default='')
            }

Then, by running:

scrapy runspider spider.py -o outputfile.csv -t csv

(or the crawl command)

you would have the following in the outputfile.csv:

Name,Year
"Faith, Love & Chocolate",2018
Bennett's Song,2018
...
Tender Mercies,1983
You Might Be the Killer,2018