Lacrit 1 #8

Lacrit · 2018-05-24T07:06:10Z

No description provided.

ssancho2

Very concise and readable!

ssancho2 · 2018-05-24T10:13:46Z

Lacrit/assignment1/q1.cpp

+#include <boost/algorithm/string.hpp>
+
+
+// input : string a , string b || output : bool 


Please move the comment to the function it applies to.

ssancho2 · 2018-05-24T10:14:02Z

Lacrit/assignment1/q1.cpp

+// input : string a , string b || output : bool 
+// determine if one is an anagram of another 
+
+std::unordered_map<char, int> count_occurences(std::string& a, const bool c)


Parameters can use better names :)
Specially bools, that could be one thing or the exact opposite.

ssancho2 · 2018-05-24T10:15:59Z

Lacrit/assignment1/q1.cpp

+	std::unordered_map<char,int> count;
+
+	//count char occurences
+	for (size_t i = 0; i < a.length(); i++)


You could do: for (char c : a) { ... } (if you rename the bool parameter, of course)

ssancho2 · 2018-05-24T10:21:10Z

Lacrit/assignment1/q1.cpp

+	std::getline(std::cin, b);
+	remove_spaces(a);
+	remove_spaces(b);
+	std::cin >> c;


NIT: You could print a line explaining to the user to enter two lines and then if he/she wants case sensitive or case insensitive anagram checking. For the last one it'll be better to use a char input, like 'c' for case sensitive and 'i' for case insensitive.

ssancho2 · 2018-05-24T10:22:30Z

Lacrit/assignment1/q1.cpp

+	remove_spaces(a);
+	remove_spaces(b);
+	std::cin >> c;
+	std::cout << isAnagram(a, b, c) << std::endl;


NIT: Similarly for the output here, a textual output like "They are anagrams"/ "They aren't anagrams". Or even "Yes" / "No" :)

ssancho2 · 2018-05-24T10:24:01Z

Lacrit/assignment1/q2.cpp

+#include <iostream>
+
+
+template<class T> 


A note on interface design, letting the user modify next freely it's risky.

ssancho2 · 2018-05-24T10:24:53Z

Lacrit/assignment1/q2.cpp

+  push(&head, 'r');
+
+  std::cout << findNode(head, 3) << std::endl;
+  return 0;


You need to free all nodes.

ssancho2 · 2018-05-24T10:26:33Z

Lacrit/assignment1/q2.cpp

+
+template<class T> 
+T findNode (Node<T> *listHead, int n)
+{


What happens if n is out of range?
What if the list is empty?

ssancho2 · 2018-05-24T10:27:02Z

Lacrit/assignment1/q2.cpp

+template<class T> 
+void push(Node<T>** head, T data)
+{
+  Node<T>* new_node =(Node<T>*) malloc(sizeof(Node<T>));


Why malloc and not new?

ssancho2

Most of the comments are style/design, and optional. The main/simple corrections are marked with '(*)'

ssancho2 · 2018-05-30T14:40:51Z

Lacrit/assignment1/q1.cpp

-	std::cout << isAnagram(a, b, c) << std::endl;
+	std::cout << "Case Sensitive?(c|i) (c => sensitive || i => insensitive)" << std::endl;
+	std::cin >> case_sensitive;
+	const std::string word = "Words aren`t anagrams";


(*) word is a weird name for the message.

ssancho2 · 2018-05-30T14:41:15Z

Lacrit/assignment1/q1.cpp

+	std::cout << "Case Sensitive?(c|i) (c => sensitive || i => insensitive)" << std::endl;
+	std::cin >> case_sensitive;
+	const std::string word = "Words aren`t anagrams";
+		remove_spaces(a);


(*) Please don't change the indentation level in the same block.

ssancho2 · 2018-05-30T14:42:13Z

Lacrit/assignment1/q1.cpp

+	std::cin >> case_sensitive;
+	const std::string word = "Words aren`t anagrams";
+		remove_spaces(a);
+		remove_spaces(b);	


I think both remove_space calls and the check if they are the same length should be part of isAnagram

ssancho2 · 2018-05-30T14:42:51Z

Lacrit/assignment1/q1.cpp

+		remove_spaces(b);	
+		if (a.length() == b.length())
+		{
+	   	    bool res = isAnagram(a, b, case_sensitive);


(*) case_sensitive is a char ('c' or 'i'), the function expects a bool.

ssancho2 · 2018-05-30T14:44:40Z

Lacrit/assignment1/q2.cpp

+		{
+			return i;
+		}
+		void set_next(Node *new_next)


The user can still modify next :)
One solution is to make a LinkedList class, and mark is as a friend of Node, that way only there you can modify it.

ssancho2 · 2018-05-30T14:45:13Z

Lacrit/assignment1/q2.cpp

 template<class T> 
 T findNode (Node<T> *listHead, int n)
 {
+	 if (listHead == NULL) return 0; 


nullptr is usually preferred to NULL, since nullptr is typed, while NULL is a int with value 0
Also, returning 0 severely limits the types T can be :)

ssancho2 · 2018-05-30T14:46:24Z

Lacrit/assignment1/q2.cpp

-  new_node->next = (*head);
+  //Node<T>* new_node =(Node<T>*) malloc(sizeof(Node<T>))
+  //Just left the upper row for reference. Correction:
+  Node<T>* new_node = new Node<T>(); // no need to pass the size


If you add a constructor to Node, you can replace this 3 lines with:
Node<T>* new_node = new Node<T>(data, *head);
Or even the whole function with:
head = new Node<T>(data, *head);
:)

ssancho2 · 2018-05-30T14:49:20Z

Lacrit/assignment1/q2.cpp

     ptr1 = ptr2 = listHead; // set the pointers to point to the list head initially

-    while(ptr1->next != NULL) // keep looping until we reach the tail (next will be NULL for the last node)
+    while(ptr1->get_next() != NULL) // keep looping until we reach the tail (next will be NULL for the last node)


What happens when n is less than 0 or bigger (or equal) to the list size?

Lacrit added 2 commits May 22, 2018 23:38

optional stuff is to be done

31f4fc9

added one func

2d09b89

ssancho2 requested changes May 24, 2018

View reviewed changes

Iteration changes

ce90388

Lacrit closed this May 29, 2018

Lacrit reopened this May 29, 2018

ssancho2 requested changes May 30, 2018

View reviewed changes

		#include <boost/algorithm/string.hpp>


		// input : string a , string b \|\| output : bool

Lacrit 1 #8

Are you sure you want to change the base?

Lacrit 1 #8

Uh oh!

Conversation

Lacrit commented May 24, 2018

Uh oh!

ssancho2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssancho2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants